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

Update documentation for indexed-zstd requirement on Apple Silcone #135

Closed
drewbitt opened this issue Apr 30, 2024 · 4 comments
Closed

Update documentation for indexed-zstd requirement on Apple Silcone #135

drewbitt opened this issue Apr 30, 2024 · 4 comments

Comments

@drewbitt
Copy link

drewbitt commented Apr 30, 2024

Originally discussed here: martinellimarco/indexed_zstd#17

You cannot install ratarmount on MacOS with just the pip command

❯ pipx install ratarmount -vvv
pipx >(setup:922): 2024-04-30 15:49:36
pipx >(setup:923): /opt/homebrew/bin/pipx install ratarmount -vvv
pipx >(setup:924): pipx version is 1.5.0
pipx >(setup:925): Default python interpreter is '/opt/homebrew/opt/python@3.12/libexec/bin/python'
pipx >(package_name_from_spec:379): Determined package name: ratarmount
pipx >(package_name_from_spec:380): Package name determined in 0.0s
creating virtual environment...
pipx >(run_subprocess:174): running /opt/homebrew/opt/python@3.12/libexec/bin/python -m venv --without-pip /Users/drewbitt/.local/pipx/venvs/ratarmount
pipx >(run_subprocess:194): stdout:
pipx >(run_subprocess:196): stderr:
pipx >(run_subprocess:197): returncode: 0
pipx >(run_subprocess:174): running <checking pip's availability>
pipx >(run_subprocess:194): stdout: ModuleSpec(name='pip', loader=<_frozen_importlib_external.SourceFileLoader object at 0x100993230>, origin='/Users/drewbitt/.local/pipx/shared/lib/python3.12/site-packages/pip/__init__.py', submodule_search_locations=['/Users/drewbitt/.local/pipx/shared/lib/python3.12/site-packages/pip'])
pipx >(run_subprocess:197): returncode: 0
pipx >(run_subprocess:174): running /Users/drewbitt/.local/pipx/venvs/ratarmount/bin/python -c import sysconfig; print(sysconfig.get_path('purelib'))
pipx >(run_subprocess:194): stdout: /Users/drewbitt/.local/pipx/venvs/ratarmount/lib/python3.12/site-packages
pipx >(run_subprocess:197): returncode: 0
pipx >(run_subprocess:174): running /Users/drewbitt/.local/pipx/shared/bin/python -c import sysconfig; print(sysconfig.get_path('purelib'))
pipx >(run_subprocess:194): stdout: /Users/drewbitt/.local/pipx/shared/lib/python3.12/site-packages
pipx >(run_subprocess:197): returncode: 0
pipx >(run_subprocess:174): running /Users/drewbitt/.local/pipx/venvs/ratarmount/bin/python --version
pipx >(run_subprocess:194): stdout: Python 3.12.3
pipx >(run_subprocess:196): stderr:
pipx >(run_subprocess:197): returncode: 0
pipx >(_parsed_package_to_package_or_url:137): cleaned package spec: ratarmount
installing ratarmount...
pipx >(run_subprocess:174): running /Users/drewbitt/.local/pipx/venvs/ratarmount/bin/python -m pip --no-input install ratarmount
pipx >(run_subprocess:197): returncode: 1
pipx >(subprocess_post_check_handle_pip_error:331): '/Users/drewbitt/.local/pipx/venvs/ratarmount/bin/python -m pip --no-input install ratarmount' failed
pipx >(subprocess_post_check_handle_pip_error:346): Fatal error from pip prevented installation. Full pip output in file:
    /Users/drewbitt/.local/pipx/logs/cmd_2024-04-30_15.49.36_pip_errors.log

pipx >(analyze_pip_output:302): pip failed to build package:
    indexed-zstd

Some possibly relevant errors from pip install:
    error: subprocess-exited-with-error
    file indexed_zstd.py (for module indexed_zstd) not found
    indexed_zstd/libzstd-seek/zstd-seek.h:20:10: fatal error: 'zstd.h' file not found
    error: command '/usr/bin/clang' failed with exit code 1

pipx >(rmdir:55): removing directory /Users/drewbitt/.local/pipx/venvs/ratarmount
Error installing ratarmount.
pipx >(cli:993): PipxError: Error installing ratarmount.
Traceback (most recent call last):
  File "/opt/homebrew/Cellar/pipx/1.5.0/libexec/lib/python3.12/site-packages/pipx/main.py", line 990, in cli
    return run_pipx_command(parsed_pipx_args, subparsers)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/Cellar/pipx/1.5.0/libexec/lib/python3.12/site-packages/pipx/main.py", line 232, in run_pipx_command
    return commands.install(
           ^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/Cellar/pipx/1.5.0/libexec/lib/python3.12/site-packages/pipx/commands/install.py", line 92, in install
    venv.install_package(
  File "/opt/homebrew/Cellar/pipx/1.5.0/libexec/lib/python3.12/site-packages/pipx/venv.py", line 263, in install_package
    raise PipxError(f"Error installing {full_package_description(package_name, package_or_url)}.")
pipx.util.PipxError: Error installing ratarmount.
pipx >(cli:1001): pipx finished.

fatal error: 'zstd.h' file not found

Solution

❯ pipx install ratarmount indexed-zstd
  installed package ratarmount 0.15.0, installed using Python 3.12.3
  These apps are now globally available
    - ratarmount
done! ✨ 🌟 ✨

I don't see this documentation about the requirement for indexed-zstd.

Alternative

Since there are wheels now for M1/M2/M3, I think we could add it back as a dependency unless there's other reasons for it's exclusion (install size etc, if other systems can install without indexed-zstd)

@mxmlnkn
Copy link
Owner

mxmlnkn commented Apr 30, 2024

Maybe it is too late in the evening already, but I am confused by these things:

  • Your solution has the same pipx call as the non-working pipx call above it. Why is it suddenly working? What is the solution you speak of in the "Solution" subheading?
  • Normally, you would have to install zstd development files via some method, which is mentioned in the readme. Therefore, I am confused what should be be done regarding the issue title: "update documentation". It is already documented.
  • In your alternative you are speaking about existing wheels. What do you mean by that? Wheels for indexed_zstd are definitely not existing (on PyPI), or else you wouldn't get the error with the missing zstd.h.
  • Why are you using pipx install ratarmount indexed-zstd instead of pipx install ratarmount?

As far as I know, the current state is:

  • The zstd.h missing error does occur when installing on non-x86-64 architectures. That's also partly why I changed macos-latest to macos-12 in the CI for now.
  • indexed_zstd only has wheels for Darwin x86-64, not for ARM.

What should be done:

  • Now that Github Actions seems to have a new macos-latest target since a few days ago, indexed_zstd should create wheels for ARM on MacOS.
  • The indexed-zstd tarball could bundle and statically link zstd. The BSD-3 license is sufficiently permissive enough. This makes it more future-proof and less cumbersome from the user side.

What could be done:

  • Alternatively, indexed_zstd as a requirement could be excluded from ratarmount specifically for Darwin and instruction could be added to facilitate manual installation if necessary.
  • I guess, the installation instructions could be written down more generically, or simply zstd could be added to the brew install macfuse call mentioned in the ReadMe. Currently, it simply shows the apt install command line because that's the system I use.

@drewbitt
Copy link
Author

drewbitt commented Apr 30, 2024

  • Your solution has the same pipx call as the non-working pipx call above it. Why is it suddenly working? What is the solution you speak of in the "Solution" subheading?

Sorry! I meant to copy the one that just was ratarmount. It had indexed_zstd with an underscore in that version, which doesnt exist, so the output should basically be the same as just ratarmount by itself. Either way, I updated it.

Normally, you would have to install zstd development files via some method, which is mentioned in the readme. Therefore, I am confused what should be be done regarding the issue title: "update documentation". It is already documented.

I have brew install zstd, which was already mentioned as not a solution martinellimarco/indexed_zstd#17 (comment). Maybe I misinterpreted that you would be aware that this is not a solution, at least for me, in being able to install ratarmount. I did have it, but still experienced the error until I had pip also install indexed-zstd.

In your alternative you are speaking about existing wheels. What do you mean by that? Wheels for indexed_zstd are definitely not existing (on PyPI), or else you wouldn't get the error with the missing zstd.h.

I can install indexed-zstd on M2.

❯ pip wheel --no-deps indexed-zstd
Collecting indexed-zstd
  Using cached indexed_zstd-1.6.0.tar.gz (66 kB)
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
  Preparing metadata (pyproject.toml) ... done
Building wheels for collected packages: indexed-zstd
  Building wheel for indexed-zstd (pyproject.toml) ... done
  Created wheel for indexed-zstd: filename=indexed_zstd-1.6.0-cp312-cp312-macosx_11_0_arm64.whl size=48534 sha256=92177ea2221af0900e8a06699ecb333b3c661635aaf1bd4048a1d48cdb500061
  Stored in directory: /Users/drewbitt/Library/Caches/pip/wheels/7b/d6/7a/795af962ac143dab3666923527a04f7b0b644efb1ce164e63b
Successfully built indexed-zstd

These will not be installed when I run pipx install ratarmount by itself; see output in first message.

Why are you using pipx install ratarmount indexed-zstd instead of pipx install ratarmount?

Because ratarmount errors with a lack of zstd.h and the one with both doesn't :)

indexed_zstd only has wheels for Darwin x86-64, not for ARM.

I see that now, so that is why I have to build them for Mac M1 and (via pipx indexed-zstd or an alternative), which is what this is all about, I guess - and noting that they are not being built with just an install of ratarmount. I am noting that I cannot install this package on a silicone mac without additional work which is not included in the README, and I advised updating the documentation. There seems to be some pushback here, so no worries, just close this issue out if you are not agreeable and it will at least exist as documentation for others.

@drewbitt drewbitt changed the title Update documentation for indexed-zstd requirement. Update documentation for indexed-zstd requirement on Apple Silcone Apr 30, 2024
@mxmlnkn
Copy link
Owner

mxmlnkn commented May 1, 2024

I have brew install zstd, which was already mentioned as not a solution martinellimarco/indexed_zstd#17 (comment). Maybe I misinterpreted that you would be aware that this is not a solution, at least for me, in being able to install ratarmount. I did have it, but still experienced the error until I had pip also install indexed-zstd.

I'm sorry, I forgot that brew install zstd didn't help as the issue is some months old, and I didn't reread the issue as far as that comment yesterday.

I still find it weird... Why would there be any difference between explicitly listing the indexed-zstd dependency and installing it as a dependency of ratarmount. It just makes no sense to me. Unfortunately, I don't have a Mac and testing via the CI is cumbersome. If you have a fix for this behavior, I'd definitely merge it, but from a user point, this almost looks like a bug in pipx... What I don't seem to see in your pipx output is which version is installed. I'd assume indexed-zstd 1.6.0 in both cases, but maybe not? The ratarmount requirement is:

    indexed_zstd >= 1.3.1, < 2.0; sys_platform=="darwin"
    indexed_zstd >= 1.2.2, < 2.0; platform_system!="Windows"

Similarly, I don't understand why your pip wheel --no-deps indexed-zstd call works but not the pip install call. Why does it find zstd.h in one case but not the other? Is something modifying the path variables?

I see that now, so that is why I have to build them for Mac M1 and (via pipx indexed-zstd or an alternative), which is what this is all about, I guess - and noting that they are not being built with just an install of ratarmount. I am noting that I cannot install this package on a silicone mac without additional work which is not included in the README, and I advised updating the documentation. There seems to be some pushback here, so no worries, just close this issue out if you are not agreeable and it will at least exist as documentation for others.

I definitely agree that users should be able to install without any errors. There is no pushback against that. I was just confused about the problem itself because I have no Mac. Simply having a discoverable issue is nice to have. In the meantime, I'll ask upstream about releasing the indexed_zstd-1.6.0-cp312-cp312-macosx_11_0_arm64.whl to PyPI. That should fix this issue, although I am still very much confused about why it installs when explicitly listed vs. simply as a dependency.

@mxmlnkn
Copy link
Owner

mxmlnkn commented May 4, 2024

Should be fixed with indexed_zstd 1.6.1

@drewbitt drewbitt closed this as completed May 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants