Skip to content

Use fallback_version in repos when git/hg is not available.#783

Closed
anntzer wants to merge 1 commit intopypa:mainfrom
anntzer:fbv
Closed

Use fallback_version in repos when git/hg is not available.#783
anntzer wants to merge 1 commit intopypa:mainfrom
anntzer:fbv

Conversation

@anntzer
Copy link
Copy Markdown
Contributor

@anntzer anntzer commented Nov 10, 2022

Closes #549.

assert res == "12.34"


def test_fallback_scm_absent(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test is deceptively incomplete as it mixes error and fallback conditions

We should trigger the fallback on error when the scm is missing

I'll have to check out the details on when to fail

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, it is not clear to me why you are unhappy about the test.
If it is that .git/.hg are not real repos, I can change that. The main point of the test is actually that making PATH empty means that subprocess.Popen will not find the git/hg executables and thus fail with a FileNotFoundError; i.e. the test behaves as if git/hg are not installed.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its not the problem that its not real repos

the problem is that the tests exercises both of them at once when setuptools_scm for the normals scm finding should more directly fail

so this should be 2 tests, one or each scm

also the scm version extraction should fail with an error and be handled instead of pretending nothing was found

the errors should bubble and be handled instead of being hidden

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so this should be 2 tests, one or each scm

Sure, I changed that, no problem.

also the scm version extraction should fail with an error and be handled instead of pretending nothing was found
the errors should bubble and be handled instead of being hidden

I thought that the behavior requested in #549 was that if the git/hg command is not present and fallback_version is set, then use fallback_version, instead of erroring out. (Certainly, that is the behavior that I would prefer. Perhaps @CharlesB2 can confirm whether that's also what he's asking for?)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the scm version finding has to fail, the failure however needs to be handled so that the fallback can happen

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAICT from
https://github.com/pypa/setuptools_scm/blob/a70d44b5621770a4d4868057a9a1503ab7ac952b/src/setuptools_scm/_entrypoints.py#L65-L69
returning None (or another falsy value) is the way for a version finder to fail.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's for not finding a version, when stuff is broken, there's need for actual errors

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand what mechanism you want me to use, then. Note that fallback_version is currently implemented as a normal entrypoint version finder, which just happens to have lowest priority and always work.
An alternative approach would be instead to special-case fallback_version and fall back to it if none of the other entrypoints succeed; I can do that if you prefer.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parse_scm is a different entrypoint than parse_scm_fallback

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see that (and can move fallback_version to the parse_scm_fallback EP if you prefer), but note again that prior to this PR nothing would catch exceptions raised by the setuptools_scm.git.parse EP and that exception would immediately bubble out; I still don't understand where you want me to handle that exception.

@RonnyPfannschmidt
Copy link
Copy Markdown
Contributor

Superseded BZ #901 which uses a different approach

@anntzer anntzer closed this Sep 13, 2023
@anntzer anntzer deleted the fbv branch September 13, 2023 22:25
arvidma pushed a commit to arvidma/setuptools_scm that referenced this pull request Sep 21, 2023
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

Successfully merging this pull request may close these issues.

Use fallback_version if SCM isn't found

2 participants