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

fs configure_file_list function #12272

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

Conversation

bruchar1
Copy link
Member

This is a new attempt to achieve the same goal as what I tried in #11822 , but in a simpler way.

This function produces, at build time, a file containing the list of files given as argument. It is used when a command uses, as argument, a file containing the list of files to process. One usecase is to provide to xgettext the list of files to process, from the list of source files, when this list is too long to be provided to the command line as individual files.

@bruchar1
Copy link
Member Author

bruchar1 commented Jun 4, 2024

Another use case is described in discussion #13235

@lhearachel
Copy link

lhearachel commented Jun 4, 2024

To wit on the above use case, our project's compiled archive binaries must match a specific SHA-sum, which means that the archives must be packed in a certain order. Historically, we would manage a separate file, but this feels brittle and does not play well with change; ideally, we would build that file from the existing list of files statically defined in the meson.build configuration, which this exactly facilitates.

@jpakkane
Copy link
Member

jpakkane commented Jun 4, 2024

Linking discussion #13235, an alternative approach would be to make custom_target use rsp files automatically. That seems to be the more general solution of the two. Would it also solve the issue this feature aims to?

@bruchar1
Copy link
Member Author

bruchar1 commented Jun 4, 2024

Linking discussion #13235, an alternative approach would be to make custom_target use rsp files automatically. That seems to be the more general solution of the two. Would it also solve the issue this feature aims to?

The problem I see is that not all custom commands can use rsp files, and not all commands use the same rsp file syntax. I don't see how this could easily be implemented in a generic way.

@lhearachel
Copy link

an alternative approach would be to make custom_target use rsp files automatically.

I do agree with @bruchar1 that it doesn't seem to make sense to automatically use rsp files, since not all executables support them nor use the same syntax. Perhaps if an executable supported a flag designating that it supports a response file, it could be standardized to dump all arguments given as input into the response file?

@jpakkane
Copy link
Member

jpakkane commented Jul 8, 2024

I do agree with @bruchar1 that it doesn't seem to make sense to automatically use rsp files, since not all executables support them nor use the same syntax.

Of course it would not be automatic, but instead have a kwarg like use_rsp_file or something like that.

@eli-schwartz
Copy link
Member

We already have a depfile kwarg, which is something else that compilers have builtin handling for but must be manually implemented for each custom target.

A possible analogous feature we could use here, would be:

custom_target(
    ...,
    command: ['myprog', '-i', '@INPUT@', '-o', '@OUTPUT@', '--foo', '--bar=baz', additional_args],
    rspfile_args: ['--args-from', '@RSPFILE@'],
    rsp_style: 'gcc',
)

Which would, I suppose, produce a commandline of myprog --args-from path/to/subdir/mytarget.rsp. Requires two kwargs instead of one and one function kwarg? Not sure what I think about either approach. Just a shower thought.

@lhearachel
Copy link

Offering support for a leading-args parameter I think makes more sense than expecting an underlying program to always interpret a response file via the same methodology. At least for my team's use case, the difference won't matter (since we build the tools where we would use a response file anyways), but it makes more sense to me to hedge against users with third-party tools that may have varying input schemae.

@jpakkane jpakkane added this to the 1.7 milestone Oct 13, 2024
@andy5995
Copy link
Contributor

@bruchar1 I'm not a lead maintainer so if you're not sure about the changes I suggested, feel free to ask someone else if they are appropriate. I had some trouble interpreting the documentation and may have suggested something that isn't intended.

This function produces, at build time, a file containing the list of
files given as argument. It is used when a command uses, as argument, a file
containing the list of files to process. One usecase is to provide to `xgettext` the
list of files to process, from the list of source files, when this list is too
long to be provided to the command line as individual files.
@bruchar1
Copy link
Member Author

@andy5995 Thank you for the review. As English is for me a second language, I may not always have the best wording for explaining the things...

I rebased and updated version numbers accordingly.

@andy5995
Copy link
Contributor

@andy5995 Thank you for the review. As English is for me a second language, I may not always have the best wording for explaining the things...

@bruchar1 You are welcome. Let me know if I can help in the future.

@jpakkane
Copy link
Member

I finally got around to going through this thoroughly. There is at least one major usability thing to consider. There are two different commonly used ways of reading command line arguments (or the like) from files. The first one is this:

process_stuff -o something --read_from inputfile.txt

But then there is also this, which is not standard but supported by many programs by convention:

process_stuff -o something @rest_of_the_command_line_arguments

The @ basically means "read the contents of the given file, parse them into args somehow and pretend that they were passed on the command line". This is typically needed on Windows because the max command line length is so limited.

Meson has native support for the latter and and it would be nice if that could be reused. But it requires the command to be run to understand the @ syntax, which many don't. So that's a problem.

The other thing is that this adds a custom_target even though all the information needed to create the input file is already present at Meson configuration time. Doing the same at configure time would be a case of running configure_file with a custom script that converts the file names given as arguments to whatever syntax your program expects.

@bruchar1
Copy link
Member Author

bruchar1 commented Nov 5, 2024

The other thing is that this adds a custom_target even though all the information needed to create the input file is already present at Meson configuration time. Doing the same at configure time would be a case of running configure_file with a custom script that converts the file names given as arguments to whatever syntax your program expects.

If I was able to pass the file names as argument to configure_file, I could simply pass them to my custom_target. The problem is that the list of file names is too big to by passed as arguments to a script.

The other thing is that even if the list of file names is known at config time, I need to detect at compile time whether any of the input files were modified. Otherwise, the custom_target wouldn't know it need to be rebuilt since the response file would be up-to-date.

@eli-schwartz
Copy link
Member

The other thing is that even if the list of file names is known at config time, I need to detect at compile time whether any of the input files were modified. Otherwise, the custom_target wouldn't know it need to be rebuilt since the response file would be up-to-date.

The reason this is a problem is because at least with the current implementation, a file_list doesn't communicate the input files as additional dependencies for the later custom_target.

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

Successfully merging this pull request may close these issues.

5 participants