RFR: JDK-8219971 Introduce SetupExecute in build system
erik.joelsson at oracle.com
Fri Mar 1 15:36:25 UTC 2019
This looks really nice! The only thing I would wish for would be that
you took some of the documentation you provided in this review mail and
added it to Execute.gmk.
Concerning the future "chmod" post commands, I think those should just
be add as something like " ; chmod $$@ " to COMMAND. The way the post
command is defined now, it's a separate rule. A simple chmod really
should be done in the same recipe as is creating the file.
On 2019-03-01 05:24, Magnus Ihse Bursie wrote:
> The most important operations performed by the build system is
> generalized into various Setup<Foo> functionality, which forms the API
> on top of which the rest of the build system is erected.
> However, we have a long tail with various small operations, calling a
> specific build tool, etc, that are done on a more ad-hoc basis. Over
> the years, a more and more refined set of "best practice" rules has
> emerged for such operations, but they tend to be implemented only for
> new operations, and not retrospectively updated on old code.
> To make sure we always keep all such operations up to date with modern
> robustness solutions, I propose to create a new, more general,
> function, SetupExecute. Basically, this takes a command line to
> execute, and make sure this is done The Right Way.
> In this patch, I'm introducing the SetupExecute function, and I've
> installed it for all locations that are currently using
> ExecuteWithLog. (Those were easy to find.) As the next step,
> operations that do not even use ExecuteWithLog (which is part of the
> "best practice" since some time ago, but was not from the start)
> should be located and converted, too.
> I have verified using COMPARE_BUILD that the resulting output does not
> change. (The solaris_sparcv9 confirmation build is still running.)
> The SetupExecute API works like this: You need to specify a COMMAND,
> the actual command line to execute. You are strongly recommended to
> provide a INFO with the text to display for LOG=info on what operation
> is performed. You can use DEPS to provide additional dependencies for
> your command to run. You can optionally include a PRE_COMMAND and a
> POST_COMMAND, intended for simple pre- and post-processing. The latter
> might be e.g. a mv from a temporary file to the final destination, the
> former e.g. a simple sed replacement on the input file. If the
> operations are unrelated to the main COMMAND, this is not a suitable
> If your command outputs a variety of files, or if it's really a single
> file but you don't really care about the output from the perspective,
> you can just supply a OUTPUT_DIR. You are supposed to make sure the
> command creates files in this directory (which will be created for you
> if it does not exist), but this can't be enforced by SetupExecute.
> Additional support files (like logs and markers) are created in this
> directory. If you want support files in a separate directory (e.g. if
> you're targeting an OUTPUT_DIR in the image directly), you can specify
> a SUPPORT_DIR. If your command outputs only a single file, you can get
> rid of the marker files (but not the log files) by specifying
> OUTPUT_FILE. Note that if you specify an OUTPUT_FILE, support log
> files will be placed in the same directory as the OUTPUT_FILE. If you
> do not want that, use SUPPORT_DIR as well.
> After the call to SetupExecute, $1 will contain references to all
> generated files (that make knows about), and $1_TARGET will contain a
> reference to the final target (that is OUTPUT_FILE if it exists, or
> the $1_exec.marker file otherwise).
> All the above keep functioning as expected even if PRE_COMMAND and
> POST_COMMAND are given. One special case worth noting is that if
> OUTPUT_FILE and POST_COMMAND is both given, the actual OUTPUT_FILE is
> considered to be a result of running the POST_COMMAND. (This will
> probably interact badly with a chmod as POST_COMMAND. I believe we
> still have a few such places, so this behavior might need to get
> revisited when I get around to converting those.)
> While this might sound a bit convoluted, I find that it matches our
> actual usage very well. The guiding idea here, as in the other
> Setup<Foo> functions, is to make the usage of the function as natural
> and simple as possible, even if at the expense of a slightly more
> complicated implementation.
> Bug: https://bugs.openjdk.java.net/browse/JDK-8219971
More information about the build-dev