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.

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

winbuild: Refactor dependency versions into constants #7843

Merged
merged 9 commits into from Mar 9, 2024

Conversation

hugovk
Copy link
Member

@hugovk hugovk commented Feb 29, 2024

Changes proposed in this pull request:

  • Move global __main__ calls into main() to avoid shadowing variables
  • Refactor version numbers into constants to reduce duplication and make updating easier

See also the last commit, I first put them as separate constants like:

BROTLI_VERSION = "1.1.0"
FREETYPE_VERSION = "2.13.2"
...

I then refactored that into a single dict:

V = {
    "BROTLI": "1.1.0",
    "FREETYPE": "2.13.2",
    ....

So we can refer to them like V['BROTLI'] instead of BROTLI_VERSION, but I'm not sure which I prefer.

Thoughts?

cc @nulano

Copy link
Contributor

@nulano nulano left a comment

Choose a reason for hiding this comment

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

Looks good.

Two suggestions:

  • Do we also want V['libimagequant'] for consistency?
  • I would also include SF_PROJECTS in the format strings now that it's not the only variable. IIRC it was done with a plus operator because it was the same length, but now it saves one character :).

winbuild/build_prepare.py Outdated Show resolved Hide resolved
winbuild/build_prepare.py Outdated Show resolved Hide resolved
winbuild/build_prepare.py Outdated Show resolved Hide resolved
winbuild/build_prepare.py Outdated Show resolved Hide resolved
Co-authored-by: Ondrej Baranovič <ondreko.tiba@gmail.com>
@hugovk
Copy link
Member Author

hugovk commented Mar 3, 2024

Two suggestions:

  • Do we also want V['libimagequant'] for consistency?

Yes, but let's do it when we're no longer pointing to a Git hash:

"libimagequant": {
# commit: Merge branch 'master' into msvc (matches 2.17.0 tag)
"url": "https://github.com/ImageOptim/libimagequant/archive/e4c1334be0eff290af5e2b4155057c2953a313ab.zip",
"filename": "libimagequant-e4c1334be0eff290af5e2b4155057c2953a313ab.zip",
"dir": "libimagequant-e4c1334be0eff290af5e2b4155057c2953a313ab",
"license": "COPYRIGHT",
"patch": {
"CMakeLists.txt": {
"if(OPENMP_FOUND)": "if(false)",
"install": "#install",
# libimagequant does not detect MSVC x86_arm64 cross-compiler correctly
"if(${{CMAKE_SYSTEM_PROCESSOR}} STREQUAL ARM64)": "if({architecture} STREQUAL ARM64)", # noqa: E501
}
},
"build": [
*cmds_cmake("imagequant_a"),
cmd_copy("imagequant_a.lib", "imagequant.lib"),
],
"headers": [r"*.h"],
"libs": [r"imagequant.lib"],
},

  • I would also include SF_PROJECTS in the format strings now that it's not the only variable. IIRC it was done with a plus operator because it was the same length, but now it saves one character :).

Good idea, suggestions applied!

@hugovk
Copy link
Member Author

hugovk commented Mar 3, 2024

I've added a couple more commits to silence IDE (PyCharm) squiggly warnings.

winbuild/build_prepare.py Outdated Show resolved Hide resolved
Co-authored-by: Andrew Murray <3112309+radarhere@users.noreply.github.com>
winbuild/build_prepare.py Outdated Show resolved Hide resolved
Co-authored-by: Andrew Murray <3112309+radarhere@users.noreply.github.com>
winbuild/build_prepare.py Outdated Show resolved Hide resolved
winbuild/build_prepare.py Outdated Show resolved Hide resolved
Co-authored-by: Andrew Murray <3112309+radarhere@users.noreply.github.com>
@radarhere radarhere merged commit 3f5721d into python-pillow:main Mar 9, 2024
81 of 84 checks passed
@hugovk hugovk deleted the refactor-winbuild branch March 9, 2024 05:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants