Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[vcpkg baseline][lensfun] Fix and cleanup #36342

Merged
merged 4 commits into from
Jan 25, 2024
Merged

[vcpkg baseline][lensfun] Fix and cleanup #36342

merged 4 commits into from
Jan 25, 2024

Conversation

dg0yt
Copy link
Contributor

@dg0yt dg0yt commented Jan 24, 2024 •
edited
Loading

Fix x64-linux baseline regression dependent on installation order.
Enable x64 optimizations.
Update license information.
Cleanup.
Remove the python module from the default features.

@MonicaLiu0311 MonicaLiu0311 self-assigned this Jan 24, 2024
@MonicaLiu0311 MonicaLiu0311 added the requires:testing Needs tests added before merging label Jan 24, 2024
@dg0yt
Copy link
Contributor Author

dg0yt commented Jan 24, 2024

Confirmed: installation order problem with port python3.

@dg0yt dg0yt changed the title [lensfun] Check build [lensfun] Fix and cleanup Jan 25, 2024
@dg0yt dg0yt mentioned this pull request Jan 25, 2024
Open
dg0yt
dg0yt commented Jan 25, 2024
View reviewed changes
ports/lensfun/vcpkg.json Outdated Show resolved Hide resolved
@MonicaLiu0311 MonicaLiu0311 added category:port-bug The issue is with a library, which is something the port should already support and removed requires:testing Needs tests added before merging labels Jan 25, 2024
@dg0yt
Copy link
Contributor Author

dg0yt commented Jan 25, 2024

Linux:

tools/python3/python3
tools/python3/python3.11

Windows:

tools/python3/python.exe

😠

@dg0yt dg0yt mentioned this pull request Jan 25, 2024
Merged
3 tasks
@dg0yt dg0yt marked this pull request as ready for review January 25, 2024 07:17
dg0yt
dg0yt commented Jan 25, 2024
View reviewed changes
ports/lensfun/vcpkg.json
"features": {
"python": {
"description": "Build python module",
"supports": "native & !windows",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Windows didn't build the python module, according to the previous CI file list, so this is not a regression. And now not possible due to #36354 (comment).

@Neumann-A
Copy link
Contributor

😠

Let me tell you there are more problems. I added the python3 binary so that scripts executed with /usr/bin/env python3 can actually find the python3 build by vcpkg.

@dg0yt
Copy link
Contributor Author

dg0yt commented Jan 25, 2024

I added the python3 binary so that scripts executed with /usr/bin/env python3 can actually find the python3 build by vcpkg.

Which is probably the cause of the installation order defects like lensfun here. As long as vcpkg.cmake sets the program path regardless of dependencies.

@MonicaLiu0311 MonicaLiu0311 changed the title [lensfun] Fix and cleanup [vcpkg baseline][lensfun] Fix and cleanup Jan 25, 2024
@BillyONeal
Copy link
Member

Thank you!

JavierMatosD
JavierMatosD approved these changes Jan 25, 2024
View reviewed changes
ports/lensfun/portfile.cmake
@@ -1,5 +1,3 @@
#vcpkg_check_linkage(ONLY_STATIC_LIBRARY)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I missed this during my review 🤦‍♂️

Thanks @dg0yt !

@JavierMatosD JavierMatosD merged commit 3b21386 into microsoft:master Jan 25, 2024
15 checks passed
@dg0yt dg0yt deleted the lensfun branch January 25, 2024 19:37
@MonicaLiu0311 MonicaLiu0311 added the info:reviewed Pull Request changes follow basic guidelines label Jan 26, 2024
TomKatom pushed a commit to TomKatom/vcpkg that referenced this pull request Feb 23, 2024
@dg0yt @TomKatom
[vcpkg baseline][lensfun] Fix and cleanup (microsoft#36342)
9b667dd 
* [lensfun] Add feature and fix python module

* [lensfun] Cleanup, fix, update license

* versions

* [lensfun] No python on windows
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JavierMatosD JavierMatosD JavierMatosD approved these changes

@BillyONeal BillyONeal BillyONeal approved these changes

Assignees

@MonicaLiu0311 MonicaLiu0311

Labels
category:port-bug The issue is with a library, which is something the port should already support info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@dg0yt @Neumann-A @BillyONeal @JavierMatosD @MonicaLiu0311