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

Push towards documented consensus on shared packages #5269

Open
becker33 opened this issue Sep 1, 2017 · 28 comments
Open

Push towards documented consensus on shared packages #5269

becker33 opened this issue Sep 1, 2017 · 28 comments

Comments

@becker33
Copy link
Member

becker33 commented Sep 1, 2017

As discussed in the telcon this week: https://github.com/LLNL/spack/wiki/Telcon%3A-2017-08-31

Also as discussed on the telcon, I'm going to use "common variants" to refer to variants that happen to be present on many packages, and "global variants" or "universal variants" to refer to a variant that is technically ensured to be present for all packages (or some major subset).

We need to document our community standards for common variants. There are reasons that we don't use universal variants for shared/static libraries, but community standards make Spack more usable. Unless/until we put those community standards into the documentation and start rejecting pull requests that don't adhere to those standards, we can't get the full benefits of standardization.

I have two concrete proposals for what the standard for shared/static variants should be. These will also interact with the (already standardized) pic variant. The goal is to hash out any corner cases of my proposals, choose one of them (or theoretically some third proposal), and document and implement it. We will then request changes to any incoming packages that fail to adhere to the standard.

Standard 1. This is what we (mostly) currently do.
All packages that can build either static or shared libraries have a variant shared. This variant defaults to True. When True, we build static and shared libraries. When false, we build only static libraries.

Pros: Already in use for most packages
Cons: Does not allow us to only build shared. Requires us to depend on ~shared or +pic versions of all dependencies when building ~static. Developers think of building "static only" more than "no shared"

Standard 2. Use multi-valued variants
All packages that can provide either static or shared libraries have a variant libraries, with possible values static and shared, non-exclusively. We provide whichever libraries are requested in this variant.

Pros: Allows any combination of shared and static libraries.
Cons: Requires changes to many packages. Requires manual variant forwarding (until #391 is reintroduced) for the libraries variant.

I'm leaning slightly towards option 2. I'd like input on possible corner cases for each of them and how they may improve or disrupt existing workflows.

@citibeth @markcmiller86 @tgamblin @alalazo @adamjstewart I know you have all had previous feelings on this topic.

@davydden
Copy link
Member

davydden commented Sep 2, 2017

If we allow building both shared and static simultaneously, can we get in trouble when within the same DAG there are packages which use both?

If not, I like multivalued variant approach.

@scheibelp
Copy link
Member

I think we were also talking about the possibility of a single-valued variant with the possible values shared, static, and static_and_shared. The idea would be that static implies building only the static libraries.

@citibeth
Copy link
Member

citibeth commented Sep 16, 2017 via email

@davydden
Copy link
Member

I think we all agree that we shall use a single variant which encodes all options. The main thing to decide is wether we allow it to be multiple choices (shared+static) or a single choice (one of shared, static, and possibly pic).

@scheibelp :

a single-valued variant with the possible values shared, static, and static_and_shared.

i am not in favour of this. If one wants to mix shared and static in one build, that's what multivalued variants for. Having static_and_shared and shared will require having all depends_on() doubled. Let's not do that.

@citibeth :

No need to try to
shoe-horn in more than one link-type in a single build.

I am ok with that. We can always revise those things if we find a specific DAG of packages which needs to have both shared and static build of one of its nodes.

single 3-valued variant (call it link-type): static, shared,
pic

I like what @citibeth suggests, specifically approach it from

how you intend to link

perspective.

In this case, maybe we only need two values: shared and static, meaning that shared will result in a package build which can be used when building shared libraries. If a package can build shared libs, then that what it should do for + shared . Otherwise it should do instead static with pic to satisfy + shared variant. Whereas static is static libraries with or without pic (that should not matter downstream).
Alternative is to name shared value dynamic.

@citibeth
Copy link
Member

citibeth commented Sep 16, 2017 via email

@davydden
Copy link
Member

davydden commented Sep 16, 2017

a single-valued variant with the possible values shared, static, and
static_and_shared.

i am not in favour of this. If one wants to mix shared and static in one
build, that's what multivalued variants for. Having static_and_shared and
shared will require having all depends_on() doubled. Let's not do that.

I agree with @scheielp here; see above for why.

I think you misunderstood my point. If there is single valued variant which can take one of {shared,static,shared_and_static} then the same choice can be encoded in a multi-valued variant {shared,static} where you need to choose at least one. But maintaining the former is a PITA as you would do

depends_on(foo+shared, when=....)
depends_on(foo+shared_and_static, when=...)

for each and every case instead of just

depends_on(foo link=shared, when=....)

which can be satisfied by both foo link=shared or foo link=shared,static.

p.s. I am not insisting that we need to adopt link={shared,static}, I am ok having link={shared,static,pic}.

@citibeth
Copy link
Member

citibeth commented Sep 16, 2017 via email

@scheibelp
Copy link
Member

If one wants to mix shared and static in one build, that's what multivalued variants for. Having static_and_shared and shared will require having all depends_on() doubled. Let's not do that.

Two points:

  • If for example a package must always build static libs, and may additionally build shared libs, then it would support {static, static_and_shared}. That's probably not a general issue though and most packages can stick with just {static, shared}
  • Also I think it's possible to add some logic to automatically propagate static/shared through dependencies so that it is not strictly required for example to say depends_on(x link=shared, when="link=shared") and depends_on(x link=static, when="link=static")

@citibeth
Copy link
Member

citibeth commented Sep 21, 2017 via email

@davydden
Copy link
Member

@scheibelp :

If for example a package must always build static libs, and may additionally build shared libs, then it would support {static, static_and_shared}.

i think you can encode this situation into a multivalued variant link={shared,static} and add a conflict statement saying that it can not have link=shared, it must be either link=shared,static or link=static. So for the sake of uniformity I would still not bother with {static, static_and_shared} approach.

@citibeth
Copy link
Member

citibeth commented Sep 21, 2017 via email

@citibeth
Copy link
Member

citibeth commented Sep 21, 2017 via email

@markcmiller86
Copy link
Contributor

Regarding @becker33 definition of common variants...

  • variants that happen to exist on many packages
  • should probably ensure they have same meanings, defaults, spellings, etc.
  • I dunno if ensure is satisfied by a manual PR checklist or by some automated tool. The latter would be better, of course.
  • would maybe have to decide when a variant is frequent enough to be deemed common and then require this additional level of management
  • Is there no hope of having a Spack-proper mechanism for defining to common variants and then ways for any package to subscribe to them? I mean something like common_variants.py file somewhere in Spack proper and then common_variant('examples') for package.py file to declare?

Regarding @becker33

Standard 1. This is what we (mostly) currently do.
All packages that can build either static or shared libraries have a variant shared. This variant defaults to True. When True, we build static and shared libraries. When false, we build only static libraries.

Just curious but how is this enforced now? In particular do all current instances indeed build both static and shared when +shared and only static with ~shared? And, what happens with pic in the latter cases? Is a separate +pic required to get static libs that can nonetheless be used in a shared link?

@becker33... What is the rationale for the con of not being able to build only shared? Is this is a disk-space issue? Time to build issue? Conflagration issue with concretization or something? Turns out, the flip side of this...not being able to build just static I am more familiar with. And, I typically arrive at this need because compilers tend to favor linking shared and then cannot due to some lib (often some darn system lib I have no control over) having been compiled without pic.

I think I like @citibeth single valued variant, link-type with choices shared, static, pic but I am not sure I understand the rationale for the name link-type? I mean I tend to think of the link step only when I am building an executable (e.g. application) and so do not immediately connote link-type with creation of a library (shared or static). If I understand it though, setting link-type say to pic means I can build all static libs but nonetheless use those libs in an eventual shared object if I need to. It seems like this variant sort of slices (think python array slicing) the spack-managed install point into objects that match one of these three criteria and feels right to me.

But, don't we already have essentially universal (@becker33 verbiage) support for pic using the picflag setting on a package? If so, why collapse that into the proposed link-type variant?

I also think a multi-valued variant with values shared and static (what about pic here?) feels better than a single-valued variant with shared, static and shared_and_static (and, again, what about pic here?)

@citibeth
Copy link
Member

citibeth commented Sep 21, 2017 via email

@scheibelp
Copy link
Member

But what's the use case for [complex logic to propagate link-types]? I'm assuming users will just set all: link-type=xxxxxx in their packages.yaml file and be done with it. Does anyone want to do anything more fine-grained than that? If so, what?

I think the primary use case for more-fine-grained setting of link types is that build/run dependencies don't need to match the link type of their parent.

I think that trying to make packages reliably build more than one link-type at a time is asking for a world of pain (see above).

we would have to go through gymnastics with every package to get the multiple link types; and the gymnastics code could come to dominate our codebase. For example, CMake natively builds only one link-type. CMakePackage would need to run the underlying CMake and build multiple times.

Even if the standardized variants are general enough to allow building both shared and static, packages should not have to support that. For example: link-type could be a multi-valued variant in general and any package could override it to be a single-valued variant; in particular CMakePackage could do that once for all CMake-based packages.

@scheibelp
Copy link
Member

Given the combinations of pic/shared/static (and moreover which ones are valid) I think that this might be better represented with two single-valued variants:

shared={none, pic, no-pic}
static={none, pic, no-pic}

@scheibelp
Copy link
Member

[one option is that] Spack guarantees that something compatible with the requested link-type (see below) is built; other link-types not requested may also be built.

Can a dependent have trouble building against a library if both the shared and static libs are present? If so IMO it is worthwhile to avoid building "extra" libraries that aren't obvious from the spec.

@citibeth
Copy link
Member

citibeth commented Sep 22, 2017 via email

@scheibelp
Copy link
Member

However... if we take an upstream package that natively builds just one link-type, and modify it to build multiple link-types, then we could conceivably create a problem for upstream packages that assumed they would find only one link-type.

My goal is not to force packages to support functionality that is implied by variant settings or combinations of them. My goal is to force the variants to accurately represent what the package builds. If a package is building extra libraries, I want it to be reflected in the variants; if it always wants to build the shared libraries for example then I don't think it should be possible set shared=False in the spec.

(still thinking on the other parts)

@alalazo
Copy link
Member

alalazo commented Sep 22, 2017

I am joining the party late, and I see that there's much fun going on 😄

Shared vs. static

I quite like @becker33 second proposal (have a common variant libraries - or libs as people will probably type this a lot - with two values shared and static). As @scheibelp noted variants can be overridden, so we can choose on a fine scale (i.e. package by package) what should be a single-valued variant and what should be a multi-valued variant. For weird cases we can use a validator function, like it is done in mvapich2 for process managers.

Position independent objects

The "picness" of the objects instead should probably be modeled with a universal boolean variant. And by universal here I really mean a single switch that can be turned on or off for all the packages at the same time. Said otherwise that should be a single global property attached to the entire DAG, not multiple and equally valued properties attached to each node. And it should enter into the hashing mechanism. I don't think we currently have support for that.

I think this property should matter only for the libs=static part - see below for details

Role of platforms

Reading the discussion I must admit my ignorance on Cray: is it impossible there to build a shared library (like it was for instance on BG/P)? If that's the case we may want to give platforms the power to inject constraints on specs. For instance a fictitious BG/P platform should be able to inject:

conflicts('libs=shared', msg='Shared libraries are not supported on BG/P')

on all specs that need it. In case, this is also a feature we currently don't have.

A few random thoughts on previous comments

Given the combinations of pic/shared/static (and moreover which ones are valid) I think that this might be better represented with two single-valued variants:

shared={none, pic, no-pic}
static={none, pic, no-pic}

Am I right saying that shared=no-pic doesn't actually exist? I ask because I lack a formal knowledge of how an OS works, but in my naive view if the code is not position independent it cannot be assigned addresses at load-time -> it cannot be shared. The case shared=none && static=none also makes no sense to me. That's what leads me to think that the model above is a better fit.

We need more discussion of the pros and cons of these two approaches. We need to think through typical user scenarios.

I would be for the opposite approach: let's go to the trenches! Out of the metaphor I think we should:

  1. select a few representative / problematic / complex packages
  2. distill a few sensible solutions from this discussion
  3. sketch all the solutions out on the packages at point 1. in different branches

I'll start proposing a few nominees: trilinos, petsc and dealii.

@davydden
Copy link
Member

I quite like @becker33 second proposal (have a common variant libraries - or libs as people will probably type this a lot - with two values shared and static)

👍

so we can choose on a fine scale (i.e. package by package) what should be a single-valued variant and what should be a multi-valued variant

👍

I would be for the opposite approach: let's go to the trenches!

fair point. petsc would probably be the easiest as it has fewer deps, whereas dealii itself depends on petsc and trilinos and from that perspective is more complex.

@adamjstewart
Copy link
Member

I haven't been following this discussion closely, but I'm fine with any solution as long as it is consistent across every package. We should be handling this at the level of AutotoolsPackage and CMakePackage if possible. That will remove a lot of complexity from individual packages.

@citibeth
Copy link
Member

citibeth commented Sep 23, 2017 via email

@davydden
Copy link
Member

davydden commented Sep 23, 2017

At one of the telcons, someone said they wanted static-PIC as an available
option. Otherwise, I'd just suggest we use shared or static.

until someone in this discussion raises concerns and explains why we really need a separate static build without pic, let's not overcomplicate the discussion and assume that we have static always built with pic. That, by the way, means that the only global setting is:

all:
  link: static

for Cray and alike. Whereas for standard systems when Spack builds shared libs, they will work with both shared and static with pic dependencies. So we don't need to enforce anything here nor forward variants in anyway. I think it would be a good balance between simplicity+robustness and complexity+features.

@citibeth
Copy link
Member

citibeth commented Sep 23, 2017 via email

@davydden
Copy link
Member

davydden commented Sep 23, 2017 via email

@bartlettroscoe
Copy link

NOTE: CMake by default can only build shared or static libraries, not both. Yes it is true you could configure two different build directories (one for shared and one for static) but that seems like a waste and you are asking for trouble installing these into the same installation directory.

Newer CMake support mulit-configuration builds but that requires usage of build generators and a lot of CMake products are not fully complaint with that yet.

@alalazo
Copy link
Member

alalazo commented May 8, 2021

Discussion revamped in #23474, where having 2 boolean variants is proposed +shared+static for a few libraries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants