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

DelocationError instead of automatically updating MacOS version of wheel #211

Closed
HexDecimal opened this issue Mar 25, 2024 · 5 comments · Fixed by #212
Closed

DelocationError instead of automatically updating MacOS version of wheel #211

HexDecimal opened this issue Mar 25, 2024 · 5 comments · Fixed by #212
Labels

Comments

@HexDecimal
Copy link
Collaborator

The MacOS comparability code added in #198 might be too strict. I had expected Delocate to derive the version automatically from its dependencies unless an explicit target version is given. In this case, MACOSX_DEPLOYMENT_TARGET is used as an explicit version when it's set and some tool has set it upstream when I didn't expect it, so it's easy to get an error such as this:

delocate-wheel --require-archs x86_64 -w /private/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/cibw-run-ddamarri/pp38-macosx_x86_64/repaired_wheel -v /private/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/cibw-run-ddamarri/pp38-macosx_x86_64/built_wheel/tcod-16.2.3.dev21+g62074a7-pp38-pypy38_pp73-macosx_10_9_x86_64.whl
  INFO:delocate.delocating:Copying library /Users/runner/work/python-tcod/python-tcod/dependencies/SDL2-2.28.1/SDL2.framework/Versions/A/SDL2 to tcod/.dylibs/SDL2
  INFO:delocate.delocating:Modifying install name in tcod/_libtcod.pypy38-pp73-darwin.so from @rpath/SDL2.framework/Versions/A/SDL2 to @loader_path/.dylibs/SDL2
  Fixing: /private/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/cibw-run-ddamarri/pp38-macosx_x86_64/built_wheel/tcod-16.2.3.dev21+g62074a7-pp38-pypy38_pp73-macosx_10_9_x86_64.whl
  Traceback (most recent call last):
    File "/private/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/cibw-run-ddamarri/pp38-macosx_x86_64/build/venv/bin/delocate-wheel", line 8, in <module>
      sys.exit(main())
    File "/private/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/cibw-run-ddamarri/pp38-macosx_x86_64/build/venv/lib/pypy3.8/site-packages/delocate/cmd/delocate_wheel.py", line 110, in main
      copied = delocate_wheel(
    File "/private/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/cibw-run-ddamarri/pp38-macosx_x86_64/build/venv/lib/pypy3.8/site-packages/delocate/delocating.py", line 1004, in delocate_wheel
      out_wheel_fixed = _check_and_update_wheel_name(
    File "/private/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/cibw-run-ddamarri/pp38-macosx_x86_64/build/venv/lib/pypy3.8/site-packages/delocate/delocating.py", line 839, in _check_and_update_wheel_name
      raise DelocationError(
  delocate.libsana.DelocationError: Library dependencies do not satisfy target MacOS version 10.9:
  /private/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/tmpahver5b_/wheel/tcod/.dylibs/SDL2 has a minimum target of 10.11

I was not expecting MACOSX_DEPLOYMENT_TARGET to behave this way since I'm unfamiliar with the variable. I had hoped that #198 would automatically handle this case by renaming the wheel, but I had to set MACOSX_DEPLOYMENT_TARGET=10.11 to resolve this.

To workaround this issue either set MACOSX_DEPLOYMENT_TARGET to a higher version or use the --require-target-macos-version <version> flag.

I could make a PR to fix this but I need to know how others feel about the MACOSX_DEPLOYMENT_TARGET variable. Maybe I only need to add instructions on how to configure the target version to this error message. Or maybe the envrioment variable should be one that's less likely to be set by upstream tools.

@HexDecimal HexDecimal added the bug label Mar 25, 2024
@matthew-brett
Copy link
Owner

Sorry for my lack of reflection - but is the question here about which workaround we should suggest to the user, of the two options a) setting e.g. export MACOSX_DEPLOYMENT_TARGET=10.11 or b) adding a delocate command line flag e.g. --require-target-macos-version 10.11?

@HexDecimal
Copy link
Collaborator Author

HexDecimal commented Mar 26, 2024

@matthew-brett My question is if both of those two workarounds should be added to the error message, or should Decloate ignore MACOSX_DEPLOYMENT_TARGET when it exists to automatically adjust the MacOS version unless --require-target-macos-version is explicitly given.

I'm leaning towards ignoring MACOSX_DEPLOYMENT_TARGET since the current behavior is breaking older scripts, but the workarounds are easy to add once they're explained. I don't know how much people value MACOSX_DEPLOYMENT_TARGET but it seemed important enough to the person who made the PR. Still, this current behavior defies what I asked for in the PR since MACOSX_DEPLOYMENT_TARGET is more pervasive than I initially thought. I did not want Delocate to fail with a version check unless --require-target-macos-version was being used.

So I need clarification on how important MACOSX_DEPLOYMENT_TARGET is for MacOS developers.

@matthew-brett
Copy link
Owner

I can't give any definitive opinion - but my guess is that MACOSX_DEPLOYMENT_TARGET is important. You probably saw, but it's very widely used on Github, and I bet it's mainly used in CI - and to specify the minimum macOS on which the binary should work. If we ignore it, then the user specifies MACOSX_DEPLOYMENT_TARGET=10.9, but in fact gets a binary that crashes on 10.9 - I bet that will often be a problem.

@bigcat88
Copy link

So I need clarification on how important MACOSX_DEPLOYMENT_TARGET is for MacOS developers.

very very large amount of packages for macOS depends on it.
Almost any package that includes C/C++ module is using it.

@HexDecimal
Copy link
Collaborator Author

I've decided to keep the current behavior and improve the error message.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants