-
Notifications
You must be signed in to change notification settings - Fork 7.1k
libx11: fix crossbuild #47648
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
base: master
Are you sure you want to change the base?
libx11: fix crossbuild #47648
Conversation
0bbd560
to
c5a8b71
Compare
Note the port is outdated. And that version is flagged with a CVE. |
Without this flag |
Tagging team review to see what direction is preferred here due to this port being outdated and wish CVE |
ports/libx11/portfile.cmake
Outdated
string(REPLACE "ac_cv_objext=" "." objsuffix "${objsuffix}") | ||
file(INSTALL "${CURRENT_BUILDTREES_DIR}/${TARGET_TRIPLET}-rel/src/util/makekeys${VCPKG_TARGET_EXECUTABLE_SUFFIX}" DESTINATION "${CURRENT_PACKAGES_DIR}/manual-tools/${PORT}") | ||
file(INSTALL "${CURRENT_BUILDTREES_DIR}/${TARGET_TRIPLET}-rel/src/util/makekeys${objsuffix}" DESTINATION "${CURRENT_PACKAGES_DIR}/manual-tools/${PORT}") | ||
file(INSTALL "${CURRENT_BUILDTREES_DIR}/${TARGET_TRIPLET}-rel/src/util/makekeys${VCPKG_TARGET_EXECUTABLE_SUFFIX}" DESTINATION "${CURRENT_PACKAGES_DIR}/manual-tools/${PORT}" USE_SOURCE_PERMISSIONS) |
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.
I believe this is meaningless as in general we don't preserve permissions. (e.g. in binary caching it goes into a zip which stores neither Unix nor Windows permissions, only the readonly bit)
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 you sure about this? Depends on the zip library used, but zip archive itself has the possibility to store executable bit: https://github.com/BOINC/boinc/blob/be83b937e72b3ddbd6f9c81994412ec37d6d4a10/zip/boinc_zip.cpp#L84-L112
If this doesn't implemented - I can do that (technically I can also check if this is really the case, because I did rebuilt of the bunch of libraries several times with the binary cache enabled, and I haven't seen this issue, but again, need to be double checked).
If there is any other way to achieve this - please let me know because this library need to have executable bit set to make successful build.
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.
Hmmmmm.... it's possible that the execute bit is preserved even if Unix permissions overall aren't. Would it be clearer to explicitly +x
rather than USE_SOURCE_PERMISSIONS
?
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.
As for me, this is an overengineering and overcomplicating, but ok.
I'll check if that works and update this PR.
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.
Or just a comment if you don't want to do that?
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.
I'll do that, no problem, not a big deal.
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.
(I originally read this as trying to do something like a system-wide install with the right permissions for a multiuser system which would be problematic as that's an explicit non-goal for vcpkg and is the origin of my 'binary caching breaks this' comment. We really Really REALLY don't want to be opening 'try to compete with apt' cans of worms and so I'm a bit paranoid when it comes to anything that may look like that if one squints. Sorry for the runaround!)
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.
No, that's completely fine, and I can completely understand you. I'm pretty much clos with the test, so I should be able to give an update of this PR within an hour.
Thank you for checking, testing, etc :)
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.
@BillyONeal, fixed.
And tested here: https://github.com/BOINC/boinc/actions/runs/18481021752/job/52655535612
Signed-off-by: Vitalii Koshura <[email protected]>
c5a8b71
to
cc6fc04
Compare
SHA512s are updated for each updated download.The "supports" clause reflects platforms that may be fixed by this new version.Any fixed CI baseline entries are removed from that file.Any patches that are no longer applied are deleted from the port's directory../vcpkg x-add-version --all
and committing the result.Only one version is added to each modified port's versions file.