waf / wscript cleanup #663
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Looks good at a quick glance, and wow that's is a nice cleanup! It's rather larger, so please allow for a few days to review it in full. |
|
Thanks. Let me know if you have questions - I might have stared at it so long that I fail to see what isn't obvious. |
|
@x42 - a gentle ping ;-) |
|
I still have a hard time to digest this. It's huge! |
New files missed in d220f47.
…top level wscript Missed in 8bb9109.
It is apparently only used in gtk2_ardour/wscript - and it seems to be enough to load it in configure(), even though it is used in build().
autowaf has no real shutdown functionality anyway. The automatic shutdown function that could have been called wouldn't work anyway, as it takes an argument. The only reason it doesn't fail is that the top level wscript has no shutdown handling and doesn't recurse to other scripts, so it is all dead code.
https://waf.io/book/ says By default, the project name and version are set to noname and 1.0. To change them, it is necessary to provide two additional variables in the top-level project file - and waf code inspection confirms that waf itself only will use the top level APPNAME.
Put it in the "children" list instead of recursing explicitly. The only difference is that the (empty) options function in libs/clearlooks-newer/wscript now will be invoked - before it wasn't.
The sub_config_and_use function recursed, but it also invoked autowaf.set_local_lib , which however didn't do anything useful. The HAVE_ defines are not used anywhere, and the AUTOWAF_LOCAL defines are only used in autowaf.use_lib, which however isn't used anywhere. Dropping these defines simplify the build environment and makes the compiler command line half as long and thus makes debugging much more manageable.
It will already have been converted to str in sanitize(), and even python2 is fine without the extra str.
CC is already set to a string. (And if it ever should be None, we want to handle that explicitly.) (And #autowaf.display_msg handle Booleans just fine.)
… applying str() all over This can perhaps be simplified further when Python2 support is dropped.
|
@pauldavisthefirst how about landing this now, after 8.0 has been released? |
|
manually rebased and merged. |
|
Alas, these changes broke the Windows builds: https://nightly.ardour.org/i/A_WIN_x86_64/build_log.txt We will try reverting "wscript: drop unnecessary str() on configure variable" |
|
Since the server's build-log is not persistent. |
|
I have reverted 615326b to get builds going again. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds consistency but shouldn't cause any functional change.
See the individual changesets and commit messages for clearest review, and cherry-pick any good parts.