-
Notifications
You must be signed in to change notification settings - Fork 824
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix compiler errors #4370
base: master
Are you sure you want to change the base?
Fix compiler errors #4370
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are the changes in SConstruct
really necessary? I read in #4243 that this was the first workaround, but the substantial changes are those in the other three files. At least this is my interpretation. I built Mapnik with only those three files changed. It builds completely, but fails many tests. Is this normal? If yes, I would approve of these changes
Yes, they are necessary. Otherwise it would fail to build, rendering mapnik un-usable for any usage. |
I was able to compile completely without the removal of Could you try to build it without changes in My scepticism is due to the thought that the flag if we just remove it, it might cause other problems that go unnoticed? The flags purpose is to set the visibility to hidden as a default value unless declared otherwise. (Explanation on SO). So usually, there is a macro to declare objects as visible, in this case it is @albertas-akistechnologies fix mentions that their fix (aka this commit) is declaring a template as visible. I don't fully understand this because I am not a programmer but even without that knowledge iI think removing the default=hidden visibility feature globally is a more drastic thing to do than to manually set the desired objects as visible. If this is not right, please help me understand! But if my reasoning is not wrong, my suggestion would be that we leave global compiler flags from |
Because this has gone stale unfortunately, could you maybe review these changes @mathisloge ? It would fix some issues, and make building easier again if you are a newbie like me and don't know the workarounds. Or do you have other plans coming in for fixing the compilation errors? |
Several issues like this (#4359 (comment)) were opened, and fixes were proposed, but no PRs were created to manage these errors without patches. So I decided to create this PR.