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

Appimage: can't install local requires directories which require extra context #1051

Closed
mhsmith opened this issue Jan 19, 2023 · 4 comments · Fixed by #1058
Closed

Appimage: can't install local requires directories which require extra context #1051

mhsmith opened this issue Jan 19, 2023 · 4 comments · Fixed by #1058
Labels
bug A crash or error in behavior. linux The issue relates Linux support.

Comments

@mhsmith
Copy link
Member

mhsmith commented Jan 19, 2023

#993 allowed AppImage to support most local requires directories by adding each one as a volume mount when running pip within Docker. However, the build will still fail if the directory's build script needs to read anything outside of the directory.

For example, see this job in the Toga setuptools-scm PR. Setuptools-scm needs to run within a Git repository in order to detect the version number. But the volume mount doesn't include the whole repository, only a subdirectory of it. This produces the error:

  × Getting requirements to build wheel did not run successfully.
  │ exit code: 1
  ╰─> [26 lines of output]
      Traceback (most recent call last):
        File "/home/brutus/.local/lib/python3.11/site-packages/pip/_vendor/pep517/in_process/_in_process.py", line 351, in <module>
          main()
        File "/home/brutus/.local/lib/python3.11/site-packages/pip/_vendor/pep517/in_process/_in_process.py", line 333, in main
          json_out['return_val'] = hook(**hook_input['kwargs'])
                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        File "/home/brutus/.local/lib/python3.11/site-packages/pip/_vendor/pep517/in_process/_in_process.py", line 118, in get_requires_for_build_wheel
          return hook(config_settings)
                 ^^^^^^^^^^^^^^^^^^^^^
        File "/tmp/pip-build-env-kp9qyb0c/overlay/local/lib/python3.11/dist-packages/setuptools/build_meta.py", line 338, in get_requires_for_build_wheel
          return self._get_build_requires(config_settings, requirements=['wheel'])
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        File "/tmp/pip-build-env-kp9qyb0c/overlay/local/lib/python3.11/dist-packages/setuptools/build_meta.py", line 320, in _get_build_requires
          self.run_setup()
        File "/tmp/pip-build-env-kp9qyb0c/overlay/local/lib/python3.11/dist-packages/setuptools/build_meta.py", line 335, in run_setup
          exec(code, locals())
        File "<string>", line 5, in <module>
        File "/tmp/pip-build-env-kp9qyb0c/overlay/local/lib/python3.11/dist-packages/setuptools_scm/__init__.py", line 148, in get_version
          _version_missing(config)
        File "/tmp/pip-build-env-kp9qyb0c/overlay/local/lib/python3.11/dist-packages/setuptools_scm/__init__.py", line 108, in _version_missing
          raise LookupError(
      LookupError: setuptools-scm was unable to detect version for /requirements.
      
      Make sure you're either building from a fully intact git repository or PyPI tarballs. Most other sources (such as GitHub's tarballs, a git checkout without the .git folder) don't contain the necessary metadata and will not work.

The simplest solution I can think of is to just mount the entire host filesystem into the Docker container, e.g. at the path /host, and use that instead of one mount per local requirement.

@mhsmith mhsmith added bug A crash or error in behavior. linux The issue relates Linux support. labels Jan 19, 2023
@freakboy3742
Copy link
Member

Well dang - that's annoying. Mounting a single /host sounds like a workable solution though, assuming there aren't any permissions problems.

@freakboy3742
Copy link
Member

Ok - I've done a quick check; it turns out to be pretty easy to implement - and as predicted, there are some permissions issues. I'm running my Docker builds on macOS, so some of these issues are macOS related - but:

  • Mounting / as /host doesn't seem to work; I can get a directory listing for /host, but none of the directories are actually readable. I'm not sure if this is because the brutus user in the container doesn't have permissions, or because there's something going on with Docker - there's a bunch of weird single-letter folders that don't actually exist on my host system, but do exist in the Docker version, plus what looks like a bunch of symlinks that can't be followed.
  • Mounting ~ as /host mostly works; but the Docker daemon pops up a warning that it will degrade system performance, and then you get asked permission to access ~/Desktop, ~/Downloads, etc. I know the situation on Linux will be different, but it seems reasonable to assume there may be some performance implications of mounting an entire drive.

Thinking around the problem - another option would be to inspect all the local package references, and specify the highest common folder as the mount point. If a project has no local folder references, then there's no /host mount at all; if there are, then there's a single /host. We then allow for an extra setting (docker_host_root - I'm open to alternative naming) which lets you specify the root folder that is used as the mounting host. If the value is manually specified, we can do a validation check that all local package references are a subfolder of that root; if the default is used, we can do a validation check that it's not overly generic (i.e., reject / and ~ as candidate roots, with a link to docs about setting docker_host_root).

This would mean:

  • Referencing a single package with no setuptools-scm style requirements should "just work" - that's effectively what we have right now.
  • Referencing multiple packages will just work, as long as all the code checkouts are in a similar subfolder (which seems likely - I don't know about you, but I've got a "beeware" folder where all my work lives)
  • The Toga case should just work (almost by accident) - specifying ../toga_core and ../toga_gtk as local packages will mean the common root is .., which will be the root of the directory that contains git information
  • In the case where it doesn't "work by accident", it's straightforward to document how to do a manual configuration.

Thoughts?

@mhsmith
Copy link
Member Author

mhsmith commented Jan 20, 2023

As discussed, a more reliable solution is probably to build the directory into an sdist, which is inherently relocatable, and then mount the sdist into the container rather than the directory. An sdist is better than a wheel, because it ensures that any native compilation is done inside the container.

We talked about using the pypa/build tool to make the sdist, but I'm not sure whether that supports building projects which have a setup.py but not a pyproject.toml.

@freakboy3742
Copy link
Member

Yes, it does - the docs for build explicitly call out the only extension to "pure" PEP517 behavior is that in the absence of a PEP517 configuration, it uses setuptools.build_meta:__legacy__. That's enough to build packages that only have setup.cfg or setup.py but no pyproject.toml (which I've tested with an old package of mine - pyspamsum - that has a setup.cfg and setup.py, but no pyproject.toml, and includes a C module).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A crash or error in behavior. linux The issue relates Linux support.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants