-
Notifications
You must be signed in to change notification settings - Fork 249
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
Update WAF to version 2.0.26 #732
Update WAF to version 2.0.26 #732
Conversation
Automatically generated build artifacts for commit c66ccdf (note: these links will expire after some time):
To test this PR, download and unzip the flatpak application above and then install with:
|
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.
./waf compiles a working hamster under debian stable only after installing python-is-python3, since debian installs the python executable only as python3.
This is probably OK since python2 is long dead.
However waf itself features a shebang of #!/bin/en/python3
, ideas?
The deletion of LICENSE.txt comes at a surprise.
Please confirm that this is OK.
I guess a PR or issue for waf project about this is an ideal solution. We can patch it locally in the mean time too.
Well, I just replaced the dir with that of waf, but maybe LICENSE file was manually added in the first place too. If it seems needed, I can add waf's license here too. Although, maybe adding waf's license out of |
Please review the following commit on master:
Is this still relevant as of today? |
Also there is this log in the history of ./waf itself:
Please check if this is still required and if yes, apply the sed to waf and force-push, please. |
Just wanted to mention that this patch allows me to build hamster on Fedora 39 and it seems to work fine so far. |
Thanks, I'll take a loop. I guess it does. But, as I said, I think it'd be better to add a license file to the top level dir instead (e.g. named
I'm not sure about this one. If someone builds Debian packages already, I guess its better for him to test, as I might not test it anytime soon. But I'll check the change to see if I've any clue why it was needed at the time, since it looks like a weird one. |
I'm not entirely sure, since a top-level file somewhat pollutes the namespace, and if upstream ever changes the wording of their license file, we might carry an older version around indefinitely. I think I'd prefer to stick to a file in What is a bit weird about the license is that the waf tarball does not have any license information in it, apart from license headers in the
The reason the binary packed version of waf is removed, is that Debian has a policy of not allowing binary/packed stuff in source tarballs, at least if source is available (see also #252 (comment)). In this case, the waf source files are available, so it does not make sense to duplicate the same code in the packed version. Regardless of Debian, I would say it also makes sense for us to have only one copy of waf in the source tree, in case we end up making modifications, having a packed copy might lead to surprises. So, for me, changes needed are:
|
Maybe also add `Closes: #731" or so to the commit message? |
There is one in PR's description. I guess it works too (?!). I'm not sure actually. :P |
What?! :P Reading the quoted commit message, I thought it has something to do with signatures or something. What's the use of packed stuff there?! Yeah, such thing is probably not allowed in many distros. OK, I'll remove it. |
c66ccdf
to
1134e8c
Compare
OK, it should be ready I think. |
Automatically generated build artifacts for commit 1134e8c (note: these links will expire after some time):
To test this PR, download and unzip the flatpak application above and then install with:
|
* Replacing ./waf & waflib/ with the latest versions in the WAF package * Removed binary waf footer; using `sed -i '/^#==>$/,$d' waf` * Added waflib/LICENSE file from project LICENSE file (currently, not available in 2.0.26, but in their source repository: https://gitlab.com/ita1024/waf/-/blob/master/LICENSE?ref_type=heads) This fixes projecthamster#731
1134e8c
to
fc787c5
Compare
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.
There is one in PR's description. I guess it works too (?!). I'm not sure actually. :P
Ah, missed that. It should work to autoclose the issue (you can preview this with the "Successfully merging this pull request may close these issues." list in the right bar in the github UI), but I prefer also putting it into the commit message, since then the git history has a direct link to the issue, instead of only via the pull request. Just a nitpick, but I've gone ahead and made this change to your branch.
I've also rebased on top of master for a cleaner merge history, force pushing to your branch.
Your other changes look good to me too!
Additionally, I've:
- Tested that without this PR, waf breaks on py3.12
- Tested that with this PR, things, waf works on py3.12
- Verified that the files added by this commit are indeed verbatim from the waf 2.0.26 tarball
- The files installed by the old waf with py3.11 are identical to the ones installed by the new waf with py3.12 (i.e. verified no behavior changed unintentionally).
So I'll merge this as soon as the CI completes.
Thanks!
Automatically generated build artifacts for commit fc787c5 (note: these links will expire after some time):
To test this PR, download and unzip the flatpak application above and then install with:
|
Just a side note: |
Personally I like meson best. |
I'm not sure it is, on https://waf.io/ the latest release is from august this year, so it seems it is still developed. Often, python software is installed from pypi using pip/pipx, which has various underlying tools, but usually those are limited to python code only, while hamster also needs dbus services, gsettings schemas, etc. I recall investigating this in the past, but now I cannot find any record of that (I thought there would be an issue about this, but I cannot find it). If we want to further discuss this, maybe create an issue for it? |
OK I will create an issue. Meson would be my favourite too but I only ever used it for C/C++. Is it any good for packaging python? |
Well, I thought PR's description will be added to merge commit's message, but apparently it doesn't happen on GitHub (it does in GitLab). Thanks anyway.
Thanks! |
Good point, I thought so as well, but it looks it only added the title to the merge commit. |
Fixes Python 3.12 compatibility
Fixes #731