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