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

[BUG]: missing version type in release #3471

Closed
2 of 3 tasks
sergiud opened this issue Nov 14, 2021 · 11 comments · Fixed by #3472
Closed
2 of 3 tasks

[BUG]: missing version type in release #3471

sergiud opened this issue Nov 14, 2021 · 11 comments · Fixed by #3472
Assignees
Labels

Comments

@sergiud
Copy link
Contributor

sergiud commented Nov 14, 2021

Required prerequisites

Problem description

When consuming pybind11 by means of find_package, the following message is generated by CMake:

-- Found pybind11: /home/sergiu/proj/devel/pybind11/install/include (found version "2.8.1" )
                                                                                          ^

Note the spurious space before the closing parenthesis. It seems that this is where the version type is supposed to appear:

"Found pybind11: ${pybind11_INCLUDE_DIR} (found version \"${pybind11_VERSION}\" ${pybind11_VERSION_TYPE})"

For some reason, however, the corresponding CMake variable is empty.

Reproducible example code

No response

@sergiud sergiud added the triage New bug, unverified label Nov 14, 2021
@Skylion007 Skylion007 added bug and removed triage New bug, unverified labels Nov 15, 2021
@Skylion007
Copy link
Collaborator

@henryiii This looks like something wrong with our release version flow using Nox. Also pinging @rwgk since he ran the release script this time.

@henryiii
Copy link
Collaborator

I could be wrong, but I thought this was supposed to be empty for release versions. The spacing is a problem, but I'm pretty sure it's supposed to be empty for releases.

@sergiud
Copy link
Contributor Author

sergiud commented Nov 15, 2021

Thanks for looking into this issue. I observe the same behavior both when I install pybind11 using plain CMake but also in the version packaged for Arch Linux using setup.py.

@henryiii
Copy link
Collaborator

I think it's because it's the correct behavior. We just haven't bothered to remove the extra space (though we probably should).

@sergiud
Copy link
Contributor Author

sergiud commented Nov 15, 2021

I see. The simplest fix would be to report the version type as part of the version string, e.g., as 2.8.1dev1. This would be consistent with how versions are generally reported by CMake in terms of find_package_handle_standard_args and maybe even use the latter instead (e.g., in CONFIG mode).

@henryiii
Copy link
Collaborator

That is what is done (though it is pulled from the version string, rather than the other way around). But for 2.8.1, there is no version type, since it's a final release. If you use master, it should be reported as dev1, which as far as I can tell, it is.

@henryiii
Copy link
Collaborator

henryiii commented Nov 15, 2021

Oh, I see what you mean, about the fix for the space. Okay, though I think the "." is not included, so you'd still have to add that.

@sergiud
Copy link
Contributor Author

sergiud commented Nov 15, 2021

As far as I can see, the version is still reported as "2.9.0" dev1. I, however, propose changing it to "2.9.0dev1". Does this make sense?

@henryiii
Copy link
Collaborator

Sorry, I'm only half here, it seems. :) Yes, that sounds reasonable and should be easy.

@henryiii
Copy link
Collaborator

Would you like to make the change?

@sergiud
Copy link
Contributor Author

sergiud commented Nov 15, 2021

Sure.

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

Successfully merging a pull request may close this issue.

3 participants