RFR: 8325963: Clean up NativeCompilation.gmk and its newly created parts

Erik Joelsson erikj at openjdk.org
Tue Feb 20 18:05:55 UTC 2024


On Thu, 15 Feb 2024 14:13:19 GMT, Magnus Ihse Bursie <ihse at openjdk.org> wrote:

> This is a follow-up to [JDK-8325877](https://bugs.openjdk.org/browse/JDK-8325877). I was careful in that bug not to change order of any lines, only split up the original file into parts.
> 
> In this PR, I use the new structure to improve the design. I collect the functionality into three clearly separate phases, preparation, compilation and linking. I use consistent naming to reveal whether a function just sets up variables, or create output. I simplify and split up a few extra hard to read functions. I move some code around so it is placed more logically.
> 
> It can be hard to convince yourself that the changes are correct if you just look at the end result. I have tried to make small and clear changes in each separate commit, and to explain in the commit title what I am doing. I recommend reviewing this PR [commit by commit](https://github.com/openjdk/jdk/pull/17873/commits).

Overall this looks good.

make/common/NativeCompilation.gmk line 132:

> 130: SetupNativeCompilation = $(NamedParamsMacroTemplate)
> 131: define SetupNativeCompilationBody
> 132:   # In this functions, helpers named Setup<Foo> are just setting variables.

Suggestion:

  # In this function, helpers named Setup<Foo> are just setting variables.

I think have tried to use the word "macro" rather than "function" in makefiles as that's what they technically are.

make/common/native/Paths.gmk line 195:

> 193: 
> 194: ################################################################################
> 195: define RemoveSuperfluousOutputFiles

This macro doesn't fit in either of the two categories mentioned at the top. Perhaps worth clarifying that it's different here. It performs direct actions on output files but also doesn't do it through a recipe.

-------------

PR Review: https://git.openjdk.org/jdk/pull/17873#pullrequestreview-1891021386
PR Review Comment: https://git.openjdk.org/jdk/pull/17873#discussion_r1496197461
PR Review Comment: https://git.openjdk.org/jdk/pull/17873#discussion_r1496208627


More information about the build-dev mailing list