Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update jlqml to new libcxxwrap / libjulia #2324

Merged
merged 2 commits into from
Jan 2, 2021

Conversation

fingolfin
Copy link
Member

Make some progress towards issue #2160

Should not be merged before @barche had a chance to review and comment. Also, there are some questions to discuss:

  1. Which version to use for this JLL? It really shouldn't be the same as right now, as the list of dependencies changes, and that requires a new version.
  2. Which Julia versions should this JLL be made compatible with? Right now, this is built for Julia 1.5.3 or later, and thus only usable in Julia >=1.5. But we could make versions supporting 1.4 or even 1.3 if desired. But that's extra work and complexity, so I first would like to get this version working, and also hear explicitly that Julia 1.3/1.4 support is wanted, before spending time on it.
  3. The two first points are actually somewhat related: in order to support multiple Julia versions, the trick we used in other JLLs is to use the patch level of the version string to indicate the Julia version; that would suggest to reversion this package to something like 0.3.X where X is equal to the one in the Julia version 1.X. However, then packages using this JLL may need to be updated.

-DQt5Svg_DIR=$prefix/lib/cmake/Qt5Svg \
-DQt5Widgets_DIR=$prefix/lib/cmake/Qt5Widgets \
$macos_extra_flags \
../jlqml/
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took the liberty of sorting these arguments alphabetically and generally aligning them with other JLLs, in the hope that this will help a bit with future maintenance.

@barche
Copy link
Contributor

barche commented Jan 1, 2021

Thanks for this, I just did a new release and updated this PR accordingly, looks perfect to me!

@fingolfin fingolfin marked this pull request as ready for review January 2, 2021 14:20
@fingolfin
Copy link
Member Author

@giordano this can be merged now

@giordano giordano merged commit 3dbfd6d into JuliaPackaging:master Jan 2, 2021
@fingolfin fingolfin deleted the mh/jlqml branch January 22, 2021 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants