manylinux icon indicating copy to clipboard operation
manylinux copied to clipboard

`versioneer`/`setuptools_scm` are unable to infer the correct version

Open brendan-ward opened this issue 3 years ago • 14 comments

We are extending the manylinux2014_x86_64 Docker image to build binary dependencies using VCPKG. This worked nicely until the latest changes in quay.io/pypa/manylinux2014_x86_64:2022-04-18-1d09d31.

Previously we were getting proper wheel names: pyogrio-0.3.0+47.g9dd1b49-cp38-cp38-linux_x86_64.whl

On latest version we now get: pyogrio-0+unknown-cp39-cp39-manylinux_2_17_x86_64.manylinux2014_x86_64.whl

We are using versioneer for managing versions, but as that was working previously, I'm not sure that is the issue here. Given that the names now include "manylinux*" it suggests that perhaps there is an issue in one of the build scripts in this latest version?

Downgrading to the quay.io/pypa/manylinux2014_x86_64:2022-04-03-da6ecb3 image fixed our issue.

xref: https://github.com/geopandas/pyogrio/pull/77

brendan-ward avatar Apr 21 '22 12:04 brendan-ward

Given that the names now include "manylinux*" it suggests that perhaps there is an issue in one of the build scripts in this latest version?

Small correction here: also on the latest docker image, we get a name like pyogrio-0+unknown-cp38-cp38-linux_x86_64.whl initially. The name including "manylinux" is the one after repairing with auditwheel.

It might be an issue with the git update?

jorisvandenbossche avatar Apr 21 '22 12:04 jorisvandenbossche

It might be an issue with the git update?

From the recent release notes:

With the fixes for CVE-2022-24765 that are common with versions of Git 2.30.4, 2.31.3, 2.32.2, 2.33.3, 2.34.3, and 2.35.3, Git has been taught not to recognise repositories owned by other users, in order to avoid getting affected by their config files and hooks. You can list the path to the safe/trusted repositories that may be owned by others on a multi-valued configuration variable safe.directory to override this behaviour, or use '*' to declare that you trust anything.

jorisvandenbossche avatar Apr 21 '22 12:04 jorisvandenbossche

Setting

git config --global --add safe.directory "*"

Within our Dockerfile appears to have resolved this issue. This may be more permissive than appropriate for all use cases, but for running on Github Actions it seemed fine. Is it appropriate to add something similar to the base Dockerfiles here?

Looks like this is not an isolated case of git upgrades breaking things in recent days...

brendan-ward avatar Apr 21 '22 20:04 brendan-ward

Thanks for the report & the analysis. I'm a bit hesitant to apply this config by default for now as it's dealing with some security issue & this just revert the security fix.

mayeut avatar Apr 24 '22 18:04 mayeut

https://github.com/pypa/cibuildwheel/pull/1095 indicates another way this can be fixed (although it might be specific to cibuildwheel, didn't look in detail)

jorisvandenbossche avatar Apr 25 '22 07:04 jorisvandenbossche

Users also can fix by being explicit, like in https://github.com/pypa/setuptools_scm/pull/708. Or by ensuring the file ownership has a consistent user all the way up (tar needs the flag in the cibuildwheel link, for example).

henryiii avatar Apr 26 '22 05:04 henryiii

I'm a bit less clear on how to solve this as a consumer of multibuild (until we migrate SciPy to cibiuldwheel)--i.e., https://github.com/MacPython/scipy-wheels/pull/167.

I tried messing around with a few different things, but each seemed to have different problems--i.e., the custom git commands would complain about permissions issues, and attempts to pin to older Docker containers seemed a bit tricky since much of the work is done upstream by multibuild.

tylerjereddy avatar Apr 30 '22 22:04 tylerjereddy

This line needs to specify the git root explicitly: https://github.com/scipy/scipy/blob/69834743023220f0073ac9565cb5783c7a2dd433/tools/version_utils.py#L84

henryiii avatar May 01 '22 03:05 henryiii

You could also just write out a version.py file in CI manually.

henryiii avatar May 01 '22 04:05 henryiii

Interesting idea, version_utils.py doesn't exist on the maintenance/1.8.x branch, but maybe I can hack the old infra in setup.py.

tylerjereddy avatar May 07 '22 22:05 tylerjereddy

Actually, even in a simple local reproducing situation I can't get the simplest fix to work really:

https://github.com/MacPython/scipy-wheels/pull/167#issuecomment-1120327126

tylerjereddy avatar May 08 '22 01:05 tylerjereddy

Can you patch the command, that is,

out = _minimal_ext_cmd(['git', "--git-dir", git_dir 'rev-parse', 'HEAD'])

I'm guessing git_dir would be something like $PWD/.git. That would fix the problem because you are no longer relying on discovery to find the git dir.

If you don't want to modify code, you should be able to set GIT_DIR=$PWD/.git.

henryiii avatar May 08 '22 01:05 henryiii

Not much luck on not modifying code--perhaps because of environment variable scoping vs. subprocess call (I'd have to modify the code to pass in a custom env to the subprocess I think).

Your code change seems promising locally though--with root owning the repo and me running as user this looks a little better (I believe):

python setup.py install

cat scipy/version.py

# THIS FILE IS GENERATED FROM SCIPY SETUP.PY
short_version = '1.8.1'
version = '1.8.1'
full_version = '1.8.1.dev0+0.a6a2fe5'
git_revision = 'a6a2fe5'
commit_count = '0'
release = False

if not release:
    version = full_version

For diff:

diff --git a/setup.py b/setup.py
index 94e72b80f..3723910f8 100755
--- a/setup.py
+++ b/setup.py
@@ -79,7 +79,9 @@ def git_version():
         return out
 
     try:
-        out = _minimal_ext_cmd(['git', 'rev-parse', 'HEAD'])
+        cwd = os.getcwd()
+        git_dir = os.path.join(cwd, ".git")
+        out = _minimal_ext_cmd(['git', '--git-dir', git_dir, 'rev-parse', 'HEAD'])
         GIT_REVISION = out.strip().decode('ascii')[:7]
 
         # We need a version number that's regularly incrementing for newer commits,

I'll give it a try in CI at least.

tylerjereddy avatar May 08 '22 02:05 tylerjereddy

@jorisvandenbossche I was able to work around this problem for NumPy, which uses versioneer and multibuild, by editing the config.sh file:

if [ $(uname) == "Linux" ]; then
    IS_LINUX=1
    ! git config --global --add safe.directory /io/numpy
fi

charris avatar May 08 '22 23:05 charris