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

zarray: New package #8717

Merged
merged 3 commits into from
May 28, 2024
Merged

Conversation

eschnett
Copy link
Contributor

No description provided.

@imciner2
Copy link
Member

So, since this package is header only, does it require the other packages there to be present when it is being used as well? e.g., if someone depends on zarray, do they need the xsimd headers as well? If so, we should make those dependencies not be a build dependency, because a build dependency is for when this package builds and not when others build.

@eschnett
Copy link
Contributor Author

xsimd is also a header-only library. Yes, to build that other package they'll neat both the (headers of) zarray and xsimd present.

There are a few other header-only packages in Yggdrasil, and they all go the BuildDependency way, e.g. the users of nlohmann-json.

I can switch over to make these regular dependencies. However, that would mean that these other packages would be downloaded at run time, and that isn't necessary.

@imciner2
Copy link
Member

Hmm, I guess this is an awkward type of transitive build dependency. I don't think we support those in BinaryBuilder yet unfortuently...

@staticfloat @giordano how about adding a new TransitiveBuildDependency type in BB2 that would add things to the build root for things higher in the chain, but not in the runtime? I guess this is really only a problem when we have header-only libraries like this though.

@staticfloat
Copy link
Member

The way I see it, if we have header-only JLL A that requires another header-only JLL B, that should not be a BuildDependency, that should be a regular dependency. Anyone who ever wants to instantiate A is going to want B as well.

If we then have C that wants A, if you include A as a BuildDependency, it will then automatically include B.

Things only get complicated when we have something like D that depends on C and needs to include those A headers; then because C required A as a BuildDependency we won't get A or B. In this case, I think it's not too crazy to just install A and B on all machines everywhere for now, it's just a few KB's extra download and on disk.

For BB2, it's possible we can have something like a TransitiveBuildDependency, I've opened an issue with some of my thoughts, but I haven't thought clearly enough to get a good final design proposed yet.

@imciner2
Copy link
Member

Things only get complicated when we have something like D that depends on C and needs to include those A headers;

Yea, this is where we have the problematic cases appearing. Looking at the dependency chain, we now have something akin to

┌────────────────┐┌─────────────┐
│xtl             ││nlohmann_json│
└┬────────┬─────┬┘└┬────────────┘
┌▽──────┐┌▽────┐│  │             
│xtensor││xsimd││  │             
└┬──────┘└┬────┘│  │             
┌▽────────▽─────▽──▽┐            
│zarray             │            
└───────────────────┘            

And with zarray being a header-only library, that means any consumers of it will have to explicitly list these 4 other libraries, or we add them as Runtime Dependencies. Listing all the dependencies seems like a massive pain going forward, so I guess the simplest option is to just make them Runtime Dependencies on the header-only libs and then the header-only libs as Build Dependencies of the final library - although they might need to be pulled in as normal dependencies even if it installs its own headers that require them.

Aren't C library dependencies fun... 🙃

@eschnett
Copy link
Contributor Author

All the libraries mentioned here are header-only libraries. There are no run-time dependencies at all.

Is there ever a need for a non-transitive BuildDependency? Tools like CMake are a HostBuildDepenceny; that's a different case.

@eschnett
Copy link
Contributor Author

Can we merge this PR? Or is there a concrete change you'd like to see?

@imciner2
Copy link
Member

IMO I think all those BuildDependency definitions should changed to Dependency to force them to be installed with this package no matter what.

@eschnett
Copy link
Contributor Author

@imciner2 Just to clarify: This means that these header-only package will then also be installed at run time although they have no binary artifacts. That's fine, I just want to be clear about this.

Should we then update other packages as well, e.g. nlohmann_json and its dependencies?

@imciner2
Copy link
Member

@imciner2 Just to clarify: This means that these header-only package will then also be installed at run time although they have no binary artifacts. That's fine, I just want to be clear about this.

Yes, they would be installed at runtime if the final consumer is using it as a Dependency. I think that is the least-worst option for this right now.

Should we then update other packages as well, e.g. nlohmann_json and its dependencies?

Yes, ideally we should update all the other libraries with this dependency system. I've merged #8777 and #8778 that do that already.

@imciner2 imciner2 merged commit 66c951d into JuliaPackaging:master May 28, 2024
4 checks passed
@eschnett eschnett deleted the eschnett/zarray branch May 28, 2024 18:43
amontoison pushed a commit that referenced this pull request Jun 22, 2024
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