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

setuptools scm workaround doesnt work for packages with dashes in the name #145

Closed
RonnyPfannschmidt opened this issue Sep 11, 2021 · 3 comments

Comments

@RonnyPfannschmidt
Copy link

the fix of #68 doesn't notify the user that it uses pretend_version,, and incorrectly puts in a bad version number

this triggers pypa/setuptools_scm#630

as https://github.com/zobayer1/logging-extras has in fact a extra dash

its ABSOLUTELY necessary to inform the users if something like SETUPTOOLS_SCM_PRETEND_VERSION is set
additionally its not safe to use for build processes that actually install dependencies (as any pulled sdist that wants setuptools_scm may get misinformed about their versions

def extract_version_from_filename(filename):
"""Extract version number from sdist filename."""
filename = os.path.splitext(os.path.basename(filename))[0]
if filename.endswith('.tar'):
filename = os.path.splitext(filename)[0]
return filename.partition('-')[2]
incorrctly extract

and then

os.environ['SETUPTOOLS_SCM_PRETEND_VERSION'] = version
always replaces without a warning (so people cant even fix it with a env var)

@zobayer1
Copy link
Contributor

I faced pypa/setuptools_scm#630 only recently. However, @RonnyPfannschmidt made a quick investigation that led me here. I am not sure whether a dash should or should not be part of a source distribution name, but it seems to be a fairly common practice to use dashes in the name. In fact, the same is done in check-manifest itself.

name='check-manifest',

As pointed out above, the code here assumes that there would not be any dashes in the file name.

return filename.partition('-')[2]

Knowing that setuptools version strings do not contain extra dashes, this could be replaced with the following

    return filename.split('-')[-1]

I have tested a build containing this change with my setup, and the tests provided with this repo. It is working as expected now.

Was there a specific reason or motivation to use the partition method there? Were there any special cases that split would not be able to handle?

@mgedmin
Copy link
Owner

mgedmin commented Sep 22, 2021

Should be fixed by #146.

@mgedmin mgedmin closed this as completed Sep 22, 2021
@mgedmin
Copy link
Owner

mgedmin commented Sep 22, 2021

Fix released in check-manifest 0.47.

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

No branches or pull requests

3 participants