RFR: 7903731: Jextract should support macOS frameworks [v3]

Maurizio Cimadamore mcimadamore at openjdk.org
Mon Jan 6 11:32:49 UTC 2025


On Thu, 2 Jan 2025 19:14:26 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`
>
> Nizar Benalla has updated the pull request incrementally with one additional commit since the last revision:
> 
>   use consistent spacing

Great work, I've left some comments for your consideration.

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

> 551:     }
> 552: 
> 553:     private Integer parseLibraries(String optionString, OptionSet optionSet, boolean useSystemLoadLibrary, Options.Builder builder) {

I'd suggest to either use `int` and then `0` for a successful execution, or something like `OptionalInt` if you want to model optionality more explicitly.

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

> 626: 
> 627:     private String formatFrameworkPath(String optionString) {
> 628:         return String.format(":/System/Library/Frameworks/%1$s.framework/%1$s", optionString);

Does this work if a custom framework dir is used? Wouldn't this absolute path point to the wrong place?

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

PR Review: https://git.openjdk.org/jextract/pull/268#pullrequestreview-2531944644
PR Review Comment: https://git.openjdk.org/jextract/pull/268#discussion_r1904038292
PR Review Comment: https://git.openjdk.org/jextract/pull/268#discussion_r1904041573


More information about the jextract-dev mailing list