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

Fix spurious error message so its clear using get_option() as a parameter to project() is prohibited #14016

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bcorby
Copy link
Contributor

@bcorby bcorby commented Dec 17, 2024

Calling get_option() from within project() fails because the meson.options file hasn’t be read yet. Reading the options file from within func_get_option fixes the issue.

@bcorby
Copy link
Contributor Author

bcorby commented Dec 17, 2024

I didn’t know where to add the test cases for this so I put them in common. I can move them.

The second commit “Add a feature fixed warning message” isn’t needed for the fix and can be dropped. If its not dropped the meson version that this fix goes in to needs to be confirmed. Its coded as 1.7 at the moment.

@bcorby bcorby marked this pull request as ready for review December 17, 2024 02:11
@bcorby bcorby requested a review from jpakkane as a code owner December 17, 2024 02:11
@eli-schwartz
Copy link
Member

  • why the second PR?
  • I wouldn't call it a fix, just a new feature... typically we use FeatureNew for this sort of thing. I don't think we've ever intended get_option() to be used in project() before, and e.g. project languages can be better solved after project(). Your test example seems confusing as the project version is project information, not something the person building decides on? (If you just want to have more flexibility when defining the version without always editing meson.build, you should probably be using version: files('VERSION.txt').)

@bcorby
Copy link
Contributor Author

bcorby commented Dec 17, 2024

I have replace the warning message with a new feature message.

My idea is to make it easier to append some local version information via a flatpak .json file. I didn’t want to be patching files but it could be done that way, just more work.

@eli-schwartz
Copy link
Member

What would that "local version information" do exactly? If this is just about getting git revision information, deriving the version from git is typically done via

project(...,
    version: run_command('./get_git_version.py', check: true).stdout().strip(),
)

It's still not generally expected to be a user-selectable option...

@bcorby
Copy link
Contributor Author

bcorby commented Dec 17, 2024

Assuming by user you mean package maintainer.

You are stating that the package maintainer should be prohibited from doing:

meson setup -Dappend_to_version=~01

because it is not generally expected?

@eli-schwartz
Copy link
Member

Assuming by user you mean package maintainer.

If you aren't 100% sure what I mean by user, then I should perhaps clarify that I am not 100% sure what you mean by package maintainer.

The maintainer of a project that uses meson as its build system may be a user of meson, but is not a "user" of the meson.build file that contains version: '1.0.0'+get_option('localversion').

@bcorby bcorby changed the title Fix the use of get_option() as a parameter to project() Fix spurious error message so its clear using get_option() as a parameter to project() is prohibited Dec 18, 2024
@bcorby bcorby force-pushed the test branch 3 times, most recently from 00802d7 to 9e5af6d Compare December 18, 2024 06:30
Make it clear that reading meson.options or a meson -Dopt=value parameter
by using get_option() as a parameter to project() isn't allowed.
@bcorby
Copy link
Contributor Author

bcorby commented Dec 18, 2024

Ok, its prohibited. On that bases I have change the code to make that clear as the current message is spurious.

I have based the new warning message on your previous suggestion. Do you think it could be improved or made more precise?

DEPRECATION: Project uses feature that was always broken, and is now deprecated since '1.7': Reading meson.options or a meson -Dopt=value parameter by using get_option() as a parameter to project() isn't allowed. Instead use something similar to run_command('./get_value.py', check: true).stdout().strip()

@bcorby
Copy link
Contributor Author

bcorby commented Dec 19, 2024

For those of you who find this and would like some more details on eli-schwartz suggestion to use run_command().

Here is a code snippet that works to replace get_option('append_to_version'). Obviously you should replace all instances of the string append_to_version with your own option.

example meson.options file:

option('append_to_version', type : 'string', value : '~01', description : 'append to version')

example meson.build file:

project('test','c',
  meson_version: '>=1.1',
  version: '1.0' + 
    # Using get_option('append_to_version') here isn't allowed by meson.
    # Instead we use a few nested run_commands.
    run_command('ps', '-p', run_command('sh', '-c', 'echo $PPID',
      check: false).stdout().strip(), '-o', 'command=', check: false).stdout()
      .split('-Dappend_to_version=').get(1,run_command('sh', '-c',
      'cat '+meson.project_source_root()/'meson.options', check: false).stdout()
      .split('append_to_version').get(1,'').split('value').get(1,'').split('\'')
      .get(1,'')).split().get(0,'')
  )

@dnicolodi
Copy link
Member

Here is a code snippet that works to replace get_option('append_to_version').

Are you trying to be funny or are you seriously considering to use code like that in a project? You seem to have completely misunderstood what @eli-schwartz is saying.

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