Skip to content

spack containerize: permit to install OS packages in the build stage#14879

Closed
alalazo wants to merge 3 commits intospack:developfrom
alalazo:features/improvements_on_containerize
Closed

spack containerize: permit to install OS packages in the build stage#14879
alalazo wants to merge 3 commits intospack:developfrom
alalazo:features/improvements_on_containerize

Conversation

@alalazo
Copy link
Copy Markdown
Member

@alalazo alalazo commented Feb 10, 2020 •
edited
Loading

refers #14802

This PR permits to install OS packages in the build stage. As such it enables the example shown in the issue above:

spack:
  specs:
  - [email protected] build_type=Release
  - [email protected] fabrics=verbs
  - fftw precision=float
  packages:
    all:
      target: [broadwell]

  container:
    format: singularity

    base:
      image: "ubuntu:18.04"
      spack: develop

    strip: true

    os_packages:
      build:
      - libibverbs-dev
      final:
      - libibverbs1
      - libgomp1

to be built.

@alalazo alalazo added feature A feature is missing in Spack containers labels Feb 10, 2020
@alalazo alalazo force-pushed the features/improvements_on_containerize branch from 0601308 to 417dfa2 Compare February 10, 2020 16:10
@alalazo
Copy link
Copy Markdown
Member Author

alalazo commented Feb 10, 2020

@samcmill this should take care of part of #14802

@tgamblin Currently I'm allowing both:

os_packages:
- lib1
- lib2

and:

os_packages:
  final:
  - lib1
  - lib2

to install OS packages in the final stage. If we want to drop the first notation the code can be simplified a bit. Let me know what you prefer.

hartzell
hartzell approved these changes Feb 11, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@hartzell hartzell left a comment

Choose a reason for hiding this comment

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

This is a useful improvement (e.g. I could load glibc-static and libstdc++-static, which I've needed to build the things in my tree (Go needed the former, can't remember what needed the latter)).

I don't have a system on which I can test this at the moment, but it looks good to my eye.

@samcmill
Copy link
Copy Markdown
Contributor

samcmill commented Feb 11, 2020

Looks good to me. Thanks for the quick fix!

Regarding the first notation, I have no objections to dropping it. If you keep it, I think it's more obvious to install the list of packages in both the build and final stages rather than just the final stage.

@alalazo
Copy link
Copy Markdown
Member Author

alalazo commented Feb 11, 2020

If you keep it, I think it's more obvious to install the list of packages in both the build and final stages rather than just the final stage.

Given that we can now install OS packages in the build stage, I completely agree. Thanks for the suggestion.

ChristianTackeGSI
ChristianTackeGSI approved these changes Feb 11, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@ChristianTackeGSI ChristianTackeGSI left a comment

Choose a reason for hiding this comment

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

I wonder, why | join(' ') doesn't work?

This was referenced Feb 13, 2020
Closed
Merged
@tgamblin tgamblin self-assigned this Mar 24, 2020
@alalazo alalazo deleted the features/improvements_on_containerize branch November 17, 2020 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@becker33 becker33 Awaiting requested review from becker33

@tgamblin tgamblin Awaiting requested review from tgamblin

2 more reviewers

@ChristianTackeGSI ChristianTackeGSI ChristianTackeGSI approved these changes

@hartzell hartzell hartzell approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@tgamblin tgamblin

Labels

containers feature A feature is missing in Spack

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@alalazo @samcmill @hartzell @ChristianTackeGSI @tgamblin @scheibelp