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

versioneer/setuptools_scm are unable to infer the correct version #1309

Closed
brendan-ward opened this issue Apr 21, 2022 · 14 comments
Closed

versioneer/setuptools_scm are unable to infer the correct version #1309

brendan-ward opened this issue Apr 21, 2022 · 14 comments
Labels

Comments

@brendan-ward
Copy link

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: geopandas/pyogrio#77

@jorisvandenbossche
Copy link

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
Copy link

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.

@brendan-ward
Copy link
Author

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...

tylerjereddy added a commit to tylerjereddy/scipy-wheels that referenced this issue Apr 23, 2022
@mayeut mayeut changed the title manylinux2014_x86_64 (2022-04-18) Docker image produces incorrect wheel names versioneer/setuptools_scm are unable to infer the correct version Apr 24, 2022
@mayeut
Copy link
Member

mayeut commented Apr 24, 2022

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 mayeut pinned this issue Apr 24, 2022
mayeut added a commit to mayeut/cibuildwheel that referenced this issue Apr 24, 2022
This allows git to work properly, e.g., `versioneer`/`setuptools_scm` with latest git versions.
c.f. pypa/manylinux#1309
@jorisvandenbossche
Copy link

pypa/cibuildwheel#1095 indicates another way this can be fixed (although it might be specific to cibuildwheel, didn't look in detail)

@henryiii
Copy link
Contributor

henryiii commented Apr 26, 2022

Users also can fix by being explicit, like in pypa/setuptools_scm#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 pushed a commit to pypa/cibuildwheel that referenced this issue Apr 26, 2022
This allows git to work properly, e.g., `versioneer`/`setuptools_scm` with latest git versions.
c.f. pypa/manylinux#1309
@tylerjereddy
Copy link

I'm a bit less clear on how to solve this as a consumer of multibuild (until we migrate SciPy to cibiuldwheel)--i.e., MacPython/scipy-wheels#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.

@henryiii
Copy link
Contributor

henryiii commented May 1, 2022

@henryiii
Copy link
Contributor

henryiii commented May 1, 2022

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

@tylerjereddy
Copy link

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
Copy link

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

MacPython/scipy-wheels#167 (comment)

@henryiii
Copy link
Contributor

henryiii commented May 8, 2022

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.

@tylerjereddy
Copy link

tylerjereddy commented May 8, 2022

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 added a commit to tylerjereddy/scipy that referenced this issue May 8, 2022
* this is an attempt to deal with the new
security measure in git:
https://github.blog/2022-04-12-git-security-vulnerability-announced/

* it has been blocking the release of SciPy 1.8.1
and NumPy point release for some time

* I'm going to try to point the problem wheels
repo PR at the hash of this commit/branch before
merging if possible:
MacPython/scipy-wheels#167

* based on feedback from Henry over here, this does
seem to help locally:
pypa/manylinux#1309 (comment)
tylerjereddy added a commit to MacPython/scipy-wheels that referenced this issue May 8, 2022
* MAINT: wheels 1.8.1 prep

* restore Pythran for Windows builds to see
if we are good to go there (if so, we can close
gh-162 as well)

* bump `BUILD_COMMIT` to point to the latest
relevant maintenance branch--this should also
tell me if anything strange is happening with
things that may be pinned since the `1.8.0` rel

* MAINT: PR 167 revisions

* try pinning setuptools for Linux jobs; the wheel
versions seem wrong with bleeding edge setuptools

* MAINT: PR 167 revisions

* try pinning `DOCKER_TEST_IMAGE` to avoid the issues related
to: https://github.blog/2022-04-12-git-security-vulnerability-announced/

* Revert "MAINT: PR 167 revisions"

This reverts commit a090151.

* MAINT: PR 167 revisions

* try using this command:
pypa/manylinux#1309 (comment)

* to deal with this in newer manylinux images:
https://github.blog/2022-04-12-git-security-vulnerability-announced/

* MAINT: PR 167 revisions

* try to address some of the issues with
`git config` commands showing up in CI

* MAINT: PR 167 revisions

* revert some `config.sh` changes that were
not helping

* MAINT: PR 167 revisions

* try shimming the `git` commands in `clean_code()`
based on feedback from Matti related to the new
`git` vulnerability fix

* DEBUG: narrow CI

* disable most of the CI while I debug

* MAINT: PR 167 revisions

* try adding the safe directory command
inside of `repo_dir`, which presumably will
include running this command in each of the
submodules

* Try workaround in scipy/scipy#16139

* MAINT: simplify after git fix.
@charris
Copy link

charris commented May 8, 2022

@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

@mayeut mayeut unpinned this issue May 14, 2022
cqc-alec added a commit to CQCL/tket that referenced this issue May 17, 2022
cqc-alec added a commit to CQCL/tket that referenced this issue May 17, 2022
chrisjbillington added a commit to chrisjbillington/desktop-app that referenced this issue Jun 20, 2022
Update others whilst we're at it why not

Some discussion here:

pypa/manylinux#1309
chrisjbillington added a commit to rpanderson/workflow-sandbox that referenced this issue Jun 20, 2022
Due to this issue:

pypa/manylinux#1309

Can likely remove the "pre-build command" once the following is merged
and released in setuptools_scm:

pypa/setuptools_scm#708

Bumping version of actions/checkout may or may not be necessary

Bumping other versions not necessary but why not
sanurielf added a commit to sanurielf/kvxopt-wheels that referenced this issue Jul 11, 2022
@mayeut mayeut closed this as completed Aug 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants