argmin-rs / argmin

Numerical optimization in pure Rust
http://argmin-rs.org
Apache License 2.0
972 stars 76 forks source link

Remove cargo-deny exception for intel-mkl-src #386

Open Tastaturtaste opened 7 months ago

Tastaturtaste commented 7 months ago

As conversed about in PR #369, this PR removes the cargo-deny exclusion for intel-mkl-src introduced due to https://github.com/EmbarkStudios/krates/issues/60.

This PR should be merged as soon as https://github.com/EmbarkStudios/krates/pull/61 lands in cargo-deny-action.

codecov-commenter commented 7 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 91.49%. Comparing base (a9e3eb7) to head (7d386c3). Report is 12 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #386 +/- ## ========================================== - Coverage 92.08% 91.49% -0.60% ========================================== Files 178 178 Lines 24455 23623 -832 ========================================== - Hits 22520 21613 -907 - Misses 1935 2010 +75 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

jedbrown commented 2 months ago

It seems this is all complete. Should this be merged now?

Tastaturtaste commented 1 month ago

It seems this is all complete. Should this be merged now?

The CI build should be rerun to confirm. If that is green it should be good to merge.

stefan-k commented 1 week ago

Would you mind rebasing this PR or allowing me to push to it? Thanks! :)

Tastaturtaste commented 1 week ago

I can rebase in the next few days. Also, permission for maintainer to edit this PR should already be given.

Tastaturtaste commented 1 week ago

Seems like cargo-deny picks up intel-mkl-src through the "examples" crates, even though those crates have "publish = false" set in their manifest and "deny.toml" has "licenses.private.ignore = true" set. If the "examples" folder is just removed from the workspace manifest members there are no license errors. I think this might not be the intended behavior for the "licenses.private.ignore" option of cargo deny. Maybe this is another bug in their implementation? I don't have the time to look closer into this right now. Maybe in the next few days, but no promises.

stefan-k commented 1 week ago

Thanks for the quick rebase, and thanks for digging into this. I've also had a look, and I suspect the path based dependencies are treated differently. I tried to replace them by workspace dependencies but to no avail. If you do look into this, please do not waste too much time on this annoying problem.

Also, permission for maintainer to edit this PR should already be given.

Interestingly I still cannot push to this branch.