-
-
Notifications
You must be signed in to change notification settings - Fork 11.3k
enable static libraries for openjpeg #49483
Conversation
system "cmake", ".", *std_cmake_args | ||
args = std_cmake_args | ||
system "cmake", ".", *args | ||
system "make", "install" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're running make install
twice; is that intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it first installs the dynamic build and then the static one. I don't think you can do them both simultaneously although I didn't try.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having to run make install
twice kind of makes me want to say this should be an option 😕.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mmm I'd really like to grab the static libraries from the bottles. I can try to run make
twice and make install
only once?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's more that not everyone will want/need the static library, and making them go through the build process twice for it feels a bit heavy-handed. I don't know. I'll let the other maintainers chime in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you ask the developers to modify their make/CMake scripts to build both with a single install
, I'd be happy to have it on by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that would be difficult for the openjpeg developers to implement in a way that works on all platforms. They would probably suggest the current solution of building the static and shared library separately as most distributions currently do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've done enough CMake to know it's not actually very hard.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've opened an issue upstream: uclouvain/openjpeg#717. I'll update this PR to make it optional.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK I updated the PR to make this optional now. Thanks for the help.
Updated this PR to make the static library optional. This should be completely harmless? |
You added some unnecessary whitespacing on the empty lines I'm afraid 😢 -
|
OK sorry about that, should be removed now. |
Merged in 5b7d569. Thank you for another contribution to Homebrew @jeroenooms! 😃. If you can get upstream to fix the requirement for a double build process we'd be happy to bottle the static library here. |
This includes the static library
libopenjpeg.a
. Should have no further side effects.