RFR: 7903731: Jextract should support macOS frameworks

[Marcono1234]

On Fri, 27 Dec 2024 17:38:32 GMT, Nizar Benalla <nbenalla at openjdk.org> wrote:

> Read the JBS issue for more detail.
> 
> This patch adds two new mac specific options, to make it easier to use frameworks and remove the need of a `compile_flags.txt` file. An exception is thrown if those options are used from an other platform, the error message is inspired from `jpackage`

The review comments below are only suggestions, feel free to ignore them, I am not a JDK member. Though hopefully the comments are useful nonetheless.

doc/GUIDE.md line 995:

> 993: | Option                       | Meaning                                                                                                                                                                                                                                 |
> 994: |:-----------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
> 995: | `--mac-framework-dir <dir>`  | specify the framework directory dir include files <br/>defaults to the current Mac OS X SDK dir<br/> This removes the need of having a compile_flags.txt with the required `-framework XYZ` options in the folder where jextract is ran |

Is this "directory dir" intentional?
Suggestion:

| `--mac-framework-dir <dir>`  | specify the framework directory include files <br/>defaults to the current Mac OS X SDK dir<br/> This removes the need of having a compile_flags.txt with the required `-framework XYZ` options in the folder where jextract is ran |

doc/GUIDE.md line 996:

> 994: |:-----------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
> 995: | `--mac-framework-dir <dir>`  | specify the framework directory dir include files <br/>defaults to the current Mac OS X SDK dir<br/> This removes the need of having a compile_flags.txt with the required `-framework XYZ` options in the folder where jextract is ran |
> 996: | `-f, framework <framekwork>` | specify the name of the library, path will be expanded to that of the framework folder                                                                                                                                                  |

Typo?
Suggestion:

| `-f, framework <framework>` | specify the name of the library, path will be expanded to that of the framework folder                                                                                                                                                  |

src/main/java/org/openjdk/jextract/JextractTool.java line 295:

> 293:         static boolean checkIfSupported( String option) {
> 294:             return platformOptions.contains(option);
> 295:         }

Maybe this name `checkIfSupported` (and the way the method is used below) is a bit misleading, the method just seems to return if this is a known platform option, so what about `isPlatformOption` instead?

src/main/java/org/openjdk/jextract/JextractTool.java line 387:

> 385:         parser.accepts("--version", "help.version", false);
> 386: 
> 387:         if(isMacOSX) {

Suggestion:

        if (isMacOSX) {

src/main/java/org/openjdk/jextract/JextractTool.java line 566:

> 564:                         builder.addLibrary(library);
> 565:                     } else {
> 566:                         // not an absolute path, but--use-system-load-library was specified

Suggestion:

                        // not an absolute path, but --use-system-load-library was specified

src/main/java/org/openjdk/jextract/JextractTool.java line 627:

> 625:     }
> 626: 
> 627:     private String parseFrameworkPath(String optionString) {

Maybe rename to `format...` since this is not parsing anything?
Suggestion:

    private String formatFrameworkPath(String optionString) {

src/main/java/org/openjdk/jextract/impl/Utils.java line 50:

> 48: import java.util.function.Consumer;
> 49: import java.util.regex.Matcher;
> 50: import java.util.regex.Pattern;

Unused imports?

src/main/resources/org/openjdk/jextract/impl/resources/Messages.properties line 93:

> 91: Platform dependent options for running jextract                                                 \n\
> 92: --mac-framework-dir <dir>          specify the framework directory                              \n\
> 93: -f <framekwork>                    specify framework library. -f libGL is equivalent to         \n\

Suggestion:

-f <framework>                    specify framework library. -f libGL is equivalent to         \n\

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

PR Review: https://git.openjdk.org/jextract/pull/268#pullrequestreview-2526080122
PR Review Comment: https://git.openjdk.org/jextract/pull/268#discussion_r1899854132
PR Review Comment: https://git.openjdk.org/jextract/pull/268#discussion_r1899854182
PR Review Comment: https://git.openjdk.org/jextract/pull/268#discussion_r1899854643
PR Review Comment: https://git.openjdk.org/jextract/pull/268#discussion_r1899854842
PR Review Comment: https://git.openjdk.org/jextract/pull/268#discussion_r1899855479
PR Review Comment: https://git.openjdk.org/jextract/pull/268#discussion_r1899855778
PR Review Comment: https://git.openjdk.org/jextract/pull/268#discussion_r1899855895
PR Review Comment: https://git.openjdk.org/jextract/pull/268#discussion_r1899856061