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

[polymake] bump to 4.2 and add patch for sigint #1695

Merged
merged 4 commits into from
Oct 9, 2020

Conversation

benlorenz
Copy link
Contributor

The sigint patch make sure that the Ctrl+C handler of julia is kept, otherwise julia will terminate immediately when ctrl+c is pressed after polymake_jll was loaded (and initialized).
Switched to archive source.

@giordano
Copy link
Member

I recommend this pull request isn't merged for a few days since it depends on GMP

@benlorenz
Copy link
Contributor Author

Ok, that's fine with me.

@fingolfin
Copy link
Member

Since PR #1739 was merged, this PR here could be "resumed" now. Not sure if that requires a rebase by @benlorenz or if it suffices to close and reopen to ensure it is rebuilt based on latest master?

@giordano
Copy link
Member

giordano commented Oct 9, 2020

CI of pull requests is run on the merge commit against the target branch

@giordano giordano closed this Oct 9, 2020
@giordano giordano reopened this Oct 9, 2020
@benlorenz benlorenz marked this pull request as ready for review October 9, 2020 09:36
]
platforms = expand_cxxstring_abis(platforms)
Copy link
Member

Choose a reason for hiding this comment

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

@giordano thanks for helping out!

@benlorenz was there a particular reason to only allow the cxx11 strings ABI (note: as far as I understand things, the cxxstring ABI does not restrict the C++ standard one can use; i.e. one can use the cxx03 string ABI also with C++11 or C++14 code, and vice versa use the cxx11 string ABI in C++03 code.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@benlorenz benlorenz Oct 9, 2020

Choose a reason for hiding this comment

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

There is no libcxxwrap_julia for the cxx03 ABI:

Linux(:i686; libc=:glibc, compiler_abi=CompilerABI(cxxstring_abi=:cxx11)),

Copy link
Member

Choose a reason for hiding this comment

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

That may hopefully change in the future

Copy link
Member

Choose a reason for hiding this comment

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

Note that a bunch of other packages are using CompilerABI, e.g. CMake, libcxxwrap_julia, libpolymake_julia, libsingular_julia, and more, so I guess all of those should be adjusted? Of course it's a bit annoying that this then means that people who want to test them locally now have to install dev versions of Julia 1.6, BinaryBuilder, and BinaryBuilderBase...

Copy link
Member

@giordano giordano Oct 9, 2020

Choose a reason for hiding this comment

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

Of course it's a bit annoying that this then means that people who want to test them locally now have to install dev versions of Julia 1.6, BinaryBuilder, and BinaryBuilderBase...

I know, but unfortunately in BinaryBuilder now we heavily rely on features present in Julia nightly which makes keeping compatibility with older Julia versions nearly impossible, or too much effort to be worth it. The compatibility shim now used for the platforms doesn't really work great, I'm not going to spend time on it (my bandwidth is limited...), but anyone is welcome to improve it .

Note that to test one of these builders locally the quick and dirty trick is to locally set

platforms = Platform[]

and manually build from the command line for whatever platform you're interested in.

@giordano giordano merged commit 1fdb02c into JuliaPackaging:master Oct 9, 2020
@benlorenz
Copy link
Contributor Author

Thanks for your help!

@benlorenz benlorenz deleted the polymake branch October 9, 2020 11:13
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