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

Migrate from BinDeps to BinaryBuilder / JLL #40

Merged
merged 7 commits into from
May 11, 2021

Conversation

bzinberg
Copy link
Contributor

@bzinberg bzinberg commented Apr 19, 2021

Summary

Fixes #39.

* Remove `BinDeps` and `Homebrew` dependencies (fixes JuliaPolyhedra#38)
* Add `lrslib_jll` dependency
* Delete `deps/` (it's no longer used)
bzinberg added a commit to bzinberg/Yggdrasil that referenced this pull request Apr 22, 2021
JuliaPolyhedra/LRSLib.jl#40 CI is passing on version [0.1.0+3](JuliaPackaging#2862) of `lrslib_jll`, and fails on earlier versions due to the lack of an `liblrsnash` LibraryProduct.  AFAIK, Julia package compatibility [specifiers](https://pkgdocs.julialang.org/v1/compatibility/) don't have a way of saying "version 0.1.0+3 or above," so we need a version bump to be able to require at least this version in LRSLib.jl's `Project.toml` (see JuliaPolyhedra/LRSLib.jl#40).  Since this release adds more API (in the form of the new LibraryProduct), I think a bump to the [minor version](https://semver.org/#spec-item-7) makes sense, i.e., to `v"0.2.0"`
@codecov
Copy link

codecov bot commented Apr 22, 2021

Codecov Report

Merging #40 (75ac477) into master (78601f8) will decrease coverage by 0.07%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #40      +/-   ##
==========================================
- Coverage   83.77%   83.70%   -0.08%     
==========================================
  Files           8        8              
  Lines         635      632       -3     
==========================================
- Hits          532      529       -3     
  Misses        103      103              
Impacted Files Coverage Δ
src/LRSLib.jl 71.42% <ø> (-8.58%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 78601f8...75ac477. Read the comment docs.

@bzinberg
Copy link
Contributor Author

This assert is failing on x86 only. @blegat, any idea what's up?

@bzinberg
Copy link
Contributor Author

Oh actually, looks like those builds were already broken. Ok, I'll call this ready for review 🙂

@bzinberg bzinberg marked this pull request as ready for review April 22, 2021 01:16
Project.toml Show resolved Hide resolved
src/polyhedron.jl Outdated Show resolved Hide resolved
Project.toml Outdated Show resolved Hide resolved
Copy link
Contributor Author

@bzinberg bzinberg left a comment

Choose a reason for hiding this comment

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

thanks

Project.toml Show resolved Hide resolved
Project.toml Outdated Show resolved Hide resolved
src/polyhedron.jl Outdated Show resolved Hide resolved
giordano pushed a commit to JuliaPackaging/Yggdrasil that referenced this pull request Apr 22, 2021
JuliaPolyhedra/LRSLib.jl#40 CI is passing on version [0.1.0+3](#2862) of `lrslib_jll`, and fails on earlier versions due to the lack of an `liblrsnash` LibraryProduct.  AFAIK, Julia package compatibility [specifiers](https://pkgdocs.julialang.org/v1/compatibility/) don't have a way of saying "version 0.1.0+3 or above," so we need a version bump to be able to require at least this version in LRSLib.jl's `Project.toml` (see JuliaPolyhedra/LRSLib.jl#40).  Since this release adds more API (in the form of the new LibraryProduct), I think a bump to the [minor version](https://semver.org/#spec-item-7) makes sense, i.e., to `v"0.2.0"`
Project.toml Outdated Show resolved Hide resolved
Copy link
Member

@blegat blegat left a comment

Choose a reason for hiding this comment

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

Thanks for the hard work, this is much appreciated!

@bzinberg
Copy link
Contributor Author

@blegat / @oyamad, could we revisit whether it's worth merging this change now, rather than waiting for the more involved changes of JuliaPolyhedra/lrslib#4? Some of my colleagues are blocked (or have to do pretty drastic workarounds) on dependency issues that boil down to this package's dependence on now-unmaintained Homebrew.jl.

@blegat
Copy link
Member

blegat commented May 11, 2021

Sorry, I thought I had merged this. Of course, it's better not to wait. This is already a major improvement over the current release

@blegat blegat merged commit 83791f7 into JuliaPolyhedra:master May 11, 2021
@bzinberg
Copy link
Contributor Author

Awesome, thanks @blegat!

@bzinberg bzinberg deleted the use-jll branch May 12, 2021 21: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.

Migrate to BinaryBuilder Homebrew failing on travis
2 participants