Redirecting input in ExternalProject_Add PATCH_COMMAND

Code
#1

In the latest code on master, if the PATCH_COMMAND includes a redirection

ExternalProject_Add( PATCH_COMMAND patch -p1 < some_file )

the < gets interpreted as an input to the command. Poking around, it looks like the underlying execute_process command should be written to use INPUT_FILE but it doesn’t look like there’s any machinery to do this. Is there an alternative way to do something like that? If not, is this a desired addtition to the ExternalProject_Add function. I’m not strong in CMake scripting but wouldn’t mind taking a crack at it.

#2

Cc: @craig.scott

#3

My recollection is that there is no guarantee that any particular shell is used to run the command, or even that a shell is used at all. Redirection may work in some cases but is not officially supported. If you need to do things like that, put it in a script and run that script as the command instead.

#4

Fair enough. It seemed to me like the INPUT_FILE argument to execute_process was meant to handle just this sort of thing.

#5

While true, it would mean CMake would need to parse out shell syntax like >foo, 2>foo, and >&foo goings on. If you want shell features, ensure you’re using the shell you want explicitly.

#6

I wonder if perhaps I have not been clear. It’s not that I’d expect CMake to understand < and the like.
Rather it’s that I’d like a way to pass down, via the ExternalProject_Add( PATCH_COMMAND ), something to feed to the INPUT_FILE argument of the execute_process, i.e. the final implementation would be something like

execute_process( COMMAND patch -p1 INPUT_FILE somefile)

as a result of

ExternalProject_Add( PATCH_COMMAND patch -p1 PATCH_INPUT somefile)

in order to get the effect of patch -p1 < somefile.

As far as I can tell, an extra argument(s) to the ExternalProject_Add would be needed, though.

Also, I’m only carrying on the argument so I can understand better. I have no trouble using a script instead.

#7

I think given the normal behavior of most patch commands, a PATCH_INPUT_FILE argument might make sense. @craig.scott? I know you’ve been refactoring in there lately, so I’m not sure how easy that’d be off-hand.

#8

I’d love to be able to make a contribution if this is work you’d like done. I’d need some guidance, but I can work fairly independently. @craig.scott’s book has been a godsend for me.

#9

Part of the complexity is that there can be more than just one command. After the PATCH_COMMAND, there could be further COMMAND blocks. Same for all the other custom command keywords like DOWNLOAD_COMMAND, UPDATE_COMMAND and so on. That means you would have to match up any INPUT_FILE with the command it was intended for. It would be even more complicated to have a keyword specific to just the PATCH_COMMAND due to the way the argument parsing is implemented, so this would most likely need to work for all custom command types, not just PATCH_COMMAND.

For the specific case in the original question, you don’t even have to use redirection. The patch command supports an -i or --input option for specifying the patch file (at least on macOS it does), so that allows you to specify an input file directly as part of the patch command.

While I’m not saying that being able to provide input redirection for commands wouldn’t be a useful enhancement, I’m wary of the complexity of implementing it and there is a well-established workaround of using a script as the command instead. The ExternalProject module is also one of the most complex of all the modules CMake provides. The way it parses arguments is… unusual, primarily because of the need to support keywords that can be repeated (the COMMAND keyword being the main culprit). It stores all keywords on targets, but only in some scenarios (FetchContent now uses it in a way that there are no targets to store them on). If you want to explore contributing this feature, perhaps take a look at the Modules/ExternalProject.cmake file as a first step. I can provide limited guidance, but my workload is already pretty high so you’d need to be fairly independent.

1 Like
#10

Well, I feel like an idiot now. I’ve used patch for years redirecting input from stdin. I even looked at the man page quickly, but missed that. Well, I certainly learned some more cmake, at least. Thanks for taking the time to respond @ben.boeckel and @craig.scott.

#11

It has become apparent that this is a regression that needs to be fixed. Shell redirections previously worked for ExternalProject, even though it was not documented to be supported. The commands were previously embedded directly in the project files and therefore were being interpreted by the underlying shell that was used. With the 3.20.0 release candidates, the commands have been getting wrapped in a CMake script that used execute_process() to invoke them, but execute_process() does not support shell redirection. You can track the relevant issue here:

https://gitlab.kitware.com/cmake/cmake/-/issues/21892

1 Like