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

add --version option to pybind11-config #4526

Merged
merged 1 commit into from
Feb 23, 2023

Conversation

eli-schwartz
Copy link
Contributor

@eli-schwartz eli-schwartz commented Feb 23, 2023

Description

Without this, it's impossible to get feature parity between detection mechanisms. Both the pkg-config file and the cmake config set their versions, but the python probe script didn't provide an option for this.

So you could print the compiler flags for using it, but you could not check what you got.

Suggested changelog entry:

`python3 -m pybind11` gained a `--version` option (prints the version and exits), for feature parity between detection mechanisms.

Copy link
Collaborator

@henryiii henryiii left a comment

Choose a reason for hiding this comment

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

Didn't realize we didn't already have this!

What about about a -v shortcut? Not as important as -h, as it sometimes is a verbosity flag. Maybe not a good idea for a new flag, just a thought.

@eli-schwartz
Copy link
Contributor Author

BTW, I need this because I'm adding a meson dependency('pybind11') that will support trying to run pybind11-config when neither pkg-config nor cmake are installed :) and this will allow us to display (and require) the version that was detected.

Didn't realize we didn't already have this!

Yeah, neither did I. :/

What about about a -v shortcut? Not as important as -h, as it sometimes is a verbosity flag. Maybe not a good idea for a new flag, just a thought.

I don't think this is necessary, no one currently depends on it and the main use case is for compatibility with classic Unix foobar-config shell scripts, which tend to have --version and not -v.

They also tend to use --cflags instead of --includes, but that is not so important, some of them also use --showme:compile or --cppflags and anyways that gets manually coded per meson dependency class.

@@ -25,6 +26,7 @@ def print_includes() -> None:

def main() -> None:
parser = argparse.ArgumentParser()
parser.add_argument("--version", action="version", version=__version__)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you please add help="Prints version and exits."

(This is almost straight from the argparse docs for action version, but I didn't know, and had to look it up.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For context, I lazily didn't bother because the argparse default is to autogenerate help text for you: "show program's version number and exit"

However I'm more than happy to make this explicit. I've tweaked your suggested wording to better fit in with the style of __main__.py which consistently uses imperative mood, so "Prints" -> "Print".

Without this, it's impossible to get feature parity between detection
mechanisms. Both the pkg-config file and the cmake config set their
versions, but the python probe script didn't provide an option for this.

So you could print the compiler flags for using it, but you could not
check what you got.
@henryiii
Copy link
Collaborator

I'm fine with sticking with --version only.

@rwgk rwgk merged commit 3cc7e42 into pybind:master Feb 23, 2023
@github-actions github-actions bot added the needs changelog Possibly needs a changelog entry label Feb 23, 2023
@rwgk
Copy link
Collaborator

rwgk commented Feb 23, 2023

@henryiii I added a Suggested changelog entry to the PR description, based on my rather superficial understanding, hoping it's a better start than nothing.

@eli-schwartz eli-schwartz deleted the pybind11-config-version branch February 23, 2023 06:36
@eli-schwartz
Copy link
Contributor Author

Thanks.

For documentation purposes, when I refer to this fix -- should I write that the fix will be in pybind11 2.10.4 or 2.11.0? I'm assuming probably the former?

@henryiii
Copy link
Collaborator

henryiii commented Feb 24, 2023

Related: do you have an example of the best way to use pybind11 & meson together (probably in the context of meson-python)?

A quick search got me to mesonbuild/meson#4677 (comment). I'm guessing this will be nicer with the dependency('pybind11')? That's something that has to be added to meson?

2.10.4 depends on if we have enough to make it worth backporting before 2.11. Don't think we are moving very quickly towards 2.11, though, so I'd say it's possible there will be a 2.10.4. I'll let know know if I know more. (If there is a 2.10.4, this should be in it)

@eli-schwartz
Copy link
Contributor Author

eli-schwartz commented Feb 24, 2023

Yes, that has to be added to meson because foobar-config --includes style scripts don't get native handling otherwise.

The idea is that people should be able to just do this:

dependency('pybind11')

and that will most likely find the pybind11-config installed to $PATH (including to the $PATH of a pip build isolation environment). Although if the global version is installed, it may find the pkg-config or cmake file instead.

@henryiii
Copy link
Collaborator

Does the meson.build file in https://github.com/scikit-hep/cookie/pull/124/files look fine? I'm assuming the stuff in the middle of it can become dependency('pybind11') later. I'm not sure if there's a better way to copy the Python files over - the current listing of each file is highly prone to break when users add a file to the Python part of the package.

@eli-schwartz
Copy link
Contributor Author

I left a comment there.

henryiii pushed a commit that referenced this pull request Feb 24, 2023
Without this, it's impossible to get feature parity between detection
mechanisms. Both the pkg-config file and the cmake config set their
versions, but the python probe script didn't provide an option for this.

So you could print the compiler flags for using it, but you could not
check what you got.
@henryiii
Copy link
Collaborator

henryiii commented Mar 9, 2023

There will be a 2.10.4, it's nearly ready except for #4301.

@henryiii
Copy link
Collaborator

Released 2.10.4.

@henryiii henryiii removed the needs changelog Possibly needs a changelog entry label Mar 16, 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.

3 participants