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

Fixes to image build process #553

Merged
merged 13 commits into from
Dec 22, 2021
Merged

Fixes to image build process #553

merged 13 commits into from
Dec 22, 2021

Conversation

csegarragonz
Copy link
Collaborator

@csegarragonz csegarragonz commented Dec 19, 2021

In this PR I fix a series of issues I have run into whilst trying to release a new docker image:

  • Pin conan dependencies to a specific revision. Otherwise, the same package versions can fail in subsequent builds if the revision for the package version changes upstream (see Pin Conan dependencies to revision faabric#200).
  • Use only virtual environments to handle python dependencies. Due to an issue with pip's editable installs, our python builds stopped working. A workaround seemed to be enabling site.ENABLE_USER_SITE in the package's setup.py (i.e. in faasmtools and faasmcli) but this derived in other problems. In the end I have decided to use only virtual environments around (as we do in regular development environments). Note that, to prevent a clash between container and out-of-container builds in the venv directory, I make out-of-container builds use venv-bm as their virtual environment root.
  • Fix linking issue in aws-sdk-cpp. This conan package seems very unstable and fragile, we revert to installing it as an ExternalProject in CMake. I add an interface library + alias to make the replacement transparent.

Closes #324

@csegarragonz csegarragonz self-assigned this Dec 19, 2021
@csegarragonz csegarragonz marked this pull request as ready for review December 21, 2021 12:47
@csegarragonz csegarragonz force-pushed the pin-conan-deps branch 11 times, most recently from 57c1d3f to bd0776d Compare December 21, 2021 15:05
@csegarragonz csegarragonz changed the title Pin Conan deps Fixes to image build process Dec 21, 2021
- name: "Build codegen - Codegen Shared Obj"
run: inv -r faasmcli/faasmcli dev.cc codegen_shared_obj
run: |
source venv/bin/activate
Copy link
Collaborator

@Shillaker Shillaker Dec 22, 2021

Choose a reason for hiding this comment

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

Can we avoid this repetition with a custom shell or by using alias for inv at the start?

I think we can put bash as the default shell too.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I set a default shell (that activates the virtual env. before running the command) only for the job that runs the tests.

Note that we can't set the shell as a default for all the workflow as not all jobs run in the same container (thus don't have the venv set, neither they need it).

Copy link
Collaborator

@Shillaker Shillaker left a comment

Choose a reason for hiding this comment

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

Awesome, nice job sorting this all out 👍

@csegarragonz csegarragonz merged commit ac05726 into master Dec 22, 2021
@csegarragonz csegarragonz deleted the pin-conan-deps branch December 22, 2021 15:37
This was referenced Feb 11, 2022
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.

Pin Python dependency versions
2 participants