JuliaPackaging / BinaryBuilderBase.jl

https://juliapackaging.github.io/BinaryBuilderBase.jl/stable
MIT License
11 stars 31 forks source link

[Prefix] Allow some dependencies not to have an `Artifacts.toml` file #278

Closed giordano closed 1 year ago

giordano commented 1 year ago

This clears warnings about MPIPreferences not having an Artifacts.toml file. One possible objection I can see is that we may not want to hardcode Yggdrasil-specific stuff in this package, could be solved with an environment variable or perhaps a preference.

vchuravy commented 1 year ago

Yeah not the biggest fan of having to manually encode these (and there might be more...), Maybe use the new top_level dependency property?

giordano commented 1 year ago

I agree we should rather check that a dependency is a top-level one, but setup_dependencies takes in a Vector{PkgSpec}, not Vector{AbstractDependency} https://github.com/JuliaPackaging/BinaryBuilderBase.jl/blob/dd8ee21b49f57bc288a48160ea3dbddb4bf284e3/src/Prefix.jl#L597-L600 so, without changing the signature of this function we can't possibly know whether a dependency was a top-level one or not. However, as I mentioned in https://github.com/JuliaPackaging/BinaryBuilderBase.jl/pull/275#discussion_r996320261 I believe these dependencies should be RuntimeDependency, rather than Dependency. I think we should fix this in Yggdrasil by changing the type of the dependency, I'm going to close this PR.