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 @BASENAMES@ and @PLAINNAMES@ to custom_target #10498

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

dcbaker
Copy link
Member

@dcbaker dcbaker commented Jun 15, 2022

There are currently projects in the wild (I don't want to shame them), who are recommending this:

custom_target(
  ...
  input : ['a.in', 'b.in', 'c.in'],
  output : '.'
  command : [find_program('cmd'), '@OUTPUT@', '@INPUT@'],
)

of course, this isn't what they actually want, they just don't want to have to list ['a.out', 'b.out', 'c.out'] in their output lines. So lets make it easy to do the right thing:

custom_target(
  ...
  input : ['a.in', 'b.in', 'c.in'],
  output : '@PLAINNAMES@.out',
  command : [find_program('cmd'), 'batch', '@OUTDIR@', '@INPUT@'],
)

With the plural form passed (it is allowed only to be passed as an exclusve output argument), meson will replace that with the @Plainname@ applied to each output, making it simple to handle a custom_target with multiple outputs that follow the @basename@ or @Plainname@ scheme.

still needs tests, but I figure we're going to bikeshed about the naming for a while, so...

This is a really bad idea, as ninja will be unable to create a reliable
DAG (since it's considering '.' (the directory) as a point in the DAG.
Unfortunately, there are projects in the wild recommending this, so we
can't just make it a hard error.
Which is useful for targets with multiple inputs, but which which the
@basename@ and @Plainname@ rules to be applied to each output.
@codecov
Copy link

codecov bot commented Jun 15, 2022

Codecov Report

Merging #10498 (258a2fb) into master (ee7a7fe) will decrease coverage by 24.82%.
The diff coverage is 39.13%.

@@             Coverage Diff             @@
##           master   #10498       +/-   ##
===========================================
- Coverage   68.74%   43.92%   -24.83%     
===========================================
  Files         406      406               
  Lines       87868    87485      -383     
  Branches    19530    18554      -976     
===========================================
- Hits        60409    38425    -21984     
- Misses      22886    45402    +22516     
+ Partials     4573     3658      -915     
Impacted Files Coverage Δ
mesonbuild/interpreter/interpreter.py 66.63% <33.33%> (-17.30%) ⬇️
mesonbuild/mesonlib/universal.py 62.30% <42.85%> (-17.74%) ⬇️
modules/qt5.py 0.00% <0.00%> (-100.00%) ⬇️
scripts/run_tool.py 0.00% <0.00%> (-100.00%) ⬇️
scripts/msgfmthelper.py 0.00% <0.00%> (-100.00%) ⬇️
mesonbuild/modules/qt5.py 0.00% <0.00%> (-100.00%) ⬇️
mesonbuild/modules/qt6.py 0.00% <0.00%> (-100.00%) ⬇️
mesonbuild/mesonlib/win32.py 0.00% <0.00%> (-100.00%) ⬇️
mesonbuild/scripts/msgfmthelper.py 0.00% <0.00%> (-100.00%) ⬇️
mesonbuild/scripts/run_tool.py 0.00% <0.00%> (-95.46%) ⬇️
... and 287 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ee7a7fe...258a2fb. Read the comment docs.

@eli-schwartz
Copy link
Member

of course, this isn't what they actually want, they just don't want to have to list ['a.out', 'b.out', 'c.out'] in their output lines. So lets make it easy to do the right thing:

Actually, these projects just want to place output files in subdirectories. It's not technically required, but the transpiler cmd in question simultaneously documents the meson snippet using output: '.' and makes the tool emit the files in directory structure. The example project using the cmd, then makes use of file aliases in the compiler to expose all those files without a directory structure at all.

In theory, the real solution here is generator(...).process(..., preserve_path_from: meson.current_source_dir()), in practice generators are totally useless unless you're outputting *.c *.cpp *.h.

@dcbaker
Copy link
Member Author

dcbaker commented Jun 16, 2022

I thought more about this, I think instead of adding a new @BASENAMES@ template, allowing @BASENAME@ to contain the array of outputs like @INPUT@ and @OUTPUT@ do would be more consistant and natural, which would also allow for @BASENAME1@, which is probably useful. This shouldn't cause any regressions because currently passing @BASENAME@ or @PLAINNAME@ with multiple outputs is an error.

@dcbaker dcbaker marked this pull request as draft June 16, 2022 17:22
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.

None yet

2 participants