On Tue, 25 Mar 2025 21:06:41 GMT, Erik Joelsson <[email protected]> wrote:
>>> Using the log file from ExecuteWithLog as the recipe target to work around >>> changing/signing the actual target file in place is an interesting choice, >>> but I think it will work. Have you tried incremental builds thoroughly? >> >> Hmm... If think you're right to be suspicious about this; I just realized >> this approach has a pretty big flaw: in the event of a failure occurring in >> the script, `make` will delete the file used as the target, which in this >> case is bad since not only the job failed, but you've just lost the log file >> that could have helped you understand why! >> To be fair, what's in the `ExecuteWithLog` file should also be present in >> `build.log`, but it will mixed up with everything else and potentially a >> pain to piece back together. >> >> I'm leaning toward reverting to my initial idea, which was to add the >> command that calls the script as part of the linking recipe. I decided to >> extract it in its own to avoid duplication, since linking for Windows and >> macOS/Linux is split across two source files, but I'm thinking that might >> not be justified, since it's only a couple of lines. >> This also has the benefit of putting to rest any worries w.r.t. incremental >> builds, I think. > >> > Using the log file from ExecuteWithLog as the recipe target to work around >> > changing/signing the actual target file in place is an interesting choice, >> > but I think it will work. Have you tried incremental builds thoroughly? >> >> Hmm... If think you're right to be suspicious about this; I just realized >> this approach has a pretty big flaw: in the event of a failure occurring in >> the script, `make` will delete the file used as the target, which in this >> case is bad since not only the job failed, but you've just lost the log file >> that could have helped you understand why! To be fair, what's in the >> `ExecuteWithLog` file should also be present in `build.log`, but it will >> mixed up with everything else and potentially a pain to piece back together. >> >> I'm leaning toward reverting to my initial idea, which was to add the >> command that calls the script as part of the linking recipe. I decided to >> extract it in its own to avoid duplication, since linking for Windows and >> macOS/Linux is split across two source files, but I'm thinking that might >> not be justified, since it's only a couple of lines. This also has the >> benefit of putting to rest any worries w.r.t. incremental builds, I think. > > In my experience there are two good ways to handle it. > > 1. Put it in the same recipe. > 2. Create a separate rule and recipe, but then the output file needs to be > different from the source file. > > 2 is generally better and what I usually go for, but it also gets more > complicated. In this case we would want the final location to stay the same, > so would need a different intermediate name or location for the linker output > when signing is enabled. Changing the name of the file is likely to cause > other downstream issues, so linking into a different (sub)directory is > probably best in that case. The script would need to take two parameters, > input file and output file. > > But I'm also good with keeping it simpler and going with 1, especially since > this isn't going to be a common operation for daily building with incremental > builds. Thanks @erikj79! I think I'd rather go with the simpler solution, as I indeed don't see a lot of practical benefit to the more sophisticated approach in this particular case. ------------- PR Comment: https://git.openjdk.org/jdk/pull/23807#issuecomment-2753717992
