Re: RFR: JDK-8244757 Introduce SetupTarget in Main.gmk
On 2020-05-12 14:52, Erik Joelsson wrote: On 2020-05-12 00:35, Magnus Ihse Bursie wrote: On 2020-05-11 19:42, Magnus Ihse Bursie wrote: On 2020-05-11 18:16, Magnus Ihse Bursie wrote: To be able to support JDK-8244410, we need to have a better way to handle dependencies. For this reason, the macro SetupTarget is introduced. It will take as input everything that is needed to create a top-level target in Main.gmk. As a positive side-effect, it will finally once again be possible to list the dependencies for a rule together with the recipe, fixing a shortcoming that has plagues Main.gmk for a long time. In this patch, I create a trivial version of SetupTarget, and switch trivial recipes to use this function. By this, I mean recipes that just delegate to another makefile, possible setting a specific target and/or make arguments. Specifically, this excludes generated target patterns, as for the module phases. Trivial dependencies are also moved into this function. By this, I mean "literal" dependencies. Dependencies that are calculated using variables or macros might change if they are moved around in the file, and need a more thorough analysis before they can be moved. My intention is to continue moving targets in Main.gmk into the scope of SetupTarget, but to avoid regressions and difficult code reviews, I will do this step by step. Bug: https://bugs.openjdk.java.net/browse/JDK-8244757 WebRev: http://cr.openjdk.java.net/~ihse/JDK-8244757-introduce-SetupTarget/webrev.01 ... aand I forgot to add the TARGET argument after I split it out from ARGS. MainSupport.gmk is updated: http://cr.openjdk.java.net/~ihse/JDK-8244757-introduce-SetupTarget/webrev.02/ And even this version was buggy. I apologize for the rushed work, it was definitely not up to my normal standard. :-( Here's yet another version. This one fixes cross-compilation. And, as Erik made me aware off-list, it adds the conditional for evaluating dependencies in SetupTarget. http://cr.openjdk.java.net/~ihse/JDK-8244757-introduce-SetupTarget/webrev.04 Third time's a charm, they say. Looks good. Missed indentation at the top of SetupTargetBody. Oops. Thanks for catching that. Fixed before pushing. /Magnus /Erik
Re: RFR: JDK-8244757 Introduce SetupTarget in Main.gmk
On 2020-05-12 00:35, Magnus Ihse Bursie wrote: On 2020-05-11 19:42, Magnus Ihse Bursie wrote: On 2020-05-11 18:16, Magnus Ihse Bursie wrote: To be able to support JDK-8244410, we need to have a better way to handle dependencies. For this reason, the macro SetupTarget is introduced. It will take as input everything that is needed to create a top-level target in Main.gmk. As a positive side-effect, it will finally once again be possible to list the dependencies for a rule together with the recipe, fixing a shortcoming that has plagues Main.gmk for a long time. In this patch, I create a trivial version of SetupTarget, and switch trivial recipes to use this function. By this, I mean recipes that just delegate to another makefile, possible setting a specific target and/or make arguments. Specifically, this excludes generated target patterns, as for the module phases. Trivial dependencies are also moved into this function. By this, I mean "literal" dependencies. Dependencies that are calculated using variables or macros might change if they are moved around in the file, and need a more thorough analysis before they can be moved. My intention is to continue moving targets in Main.gmk into the scope of SetupTarget, but to avoid regressions and difficult code reviews, I will do this step by step. Bug: https://bugs.openjdk.java.net/browse/JDK-8244757 WebRev: http://cr.openjdk.java.net/~ihse/JDK-8244757-introduce-SetupTarget/webrev.01 ... aand I forgot to add the TARGET argument after I split it out from ARGS. MainSupport.gmk is updated: http://cr.openjdk.java.net/~ihse/JDK-8244757-introduce-SetupTarget/webrev.02/ And even this version was buggy. I apologize for the rushed work, it was definitely not up to my normal standard. :-( Here's yet another version. This one fixes cross-compilation. And, as Erik made me aware off-list, it adds the conditional for evaluating dependencies in SetupTarget. http://cr.openjdk.java.net/~ihse/JDK-8244757-introduce-SetupTarget/webrev.04 Third time's a charm, they say. Looks good. Missed indentation at the top of SetupTargetBody. /Erik
Re: RFR: JDK-8244757 Introduce SetupTarget in Main.gmk
On 2020-05-11 19:42, Magnus Ihse Bursie wrote: On 2020-05-11 18:16, Magnus Ihse Bursie wrote: To be able to support JDK-8244410, we need to have a better way to handle dependencies. For this reason, the macro SetupTarget is introduced. It will take as input everything that is needed to create a top-level target in Main.gmk. As a positive side-effect, it will finally once again be possible to list the dependencies for a rule together with the recipe, fixing a shortcoming that has plagues Main.gmk for a long time. In this patch, I create a trivial version of SetupTarget, and switch trivial recipes to use this function. By this, I mean recipes that just delegate to another makefile, possible setting a specific target and/or make arguments. Specifically, this excludes generated target patterns, as for the module phases. Trivial dependencies are also moved into this function. By this, I mean "literal" dependencies. Dependencies that are calculated using variables or macros might change if they are moved around in the file, and need a more thorough analysis before they can be moved. My intention is to continue moving targets in Main.gmk into the scope of SetupTarget, but to avoid regressions and difficult code reviews, I will do this step by step. Bug: https://bugs.openjdk.java.net/browse/JDK-8244757 WebRev: http://cr.openjdk.java.net/~ihse/JDK-8244757-introduce-SetupTarget/webrev.01 ... aand I forgot to add the TARGET argument after I split it out from ARGS. MainSupport.gmk is updated: http://cr.openjdk.java.net/~ihse/JDK-8244757-introduce-SetupTarget/webrev.02/ And even this version was buggy. I apologize for the rushed work, it was definitely not up to my normal standard. :-( Here's yet another version. This one fixes cross-compilation. And, as Erik made me aware off-list, it adds the conditional for evaluating dependencies in SetupTarget. http://cr.openjdk.java.net/~ihse/JDK-8244757-introduce-SetupTarget/webrev.04 Third time's a charm, they say. /Magnus /Magnus /Magnus
Re: RFR: JDK-8244757 Introduce SetupTarget in Main.gmk
On 2020-05-11 18:16, Magnus Ihse Bursie wrote: To be able to support JDK-8244410, we need to have a better way to handle dependencies. For this reason, the macro SetupTarget is introduced. It will take as input everything that is needed to create a top-level target in Main.gmk. As a positive side-effect, it will finally once again be possible to list the dependencies for a rule together with the recipe, fixing a shortcoming that has plagues Main.gmk for a long time. In this patch, I create a trivial version of SetupTarget, and switch trivial recipes to use this function. By this, I mean recipes that just delegate to another makefile, possible setting a specific target and/or make arguments. Specifically, this excludes generated target patterns, as for the module phases. Trivial dependencies are also moved into this function. By this, I mean "literal" dependencies. Dependencies that are calculated using variables or macros might change if they are moved around in the file, and need a more thorough analysis before they can be moved. My intention is to continue moving targets in Main.gmk into the scope of SetupTarget, but to avoid regressions and difficult code reviews, I will do this step by step. Bug: https://bugs.openjdk.java.net/browse/JDK-8244757 WebRev: http://cr.openjdk.java.net/~ihse/JDK-8244757-introduce-SetupTarget/webrev.01 ... aand I forgot to add the TARGET argument after I split it out from ARGS. MainSupport.gmk is updated: http://cr.openjdk.java.net/~ihse/JDK-8244757-introduce-SetupTarget/webrev.02/ /Magnus /Magnus
