Skip to content

Use Scons in CI with container #959

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

Merged
merged 38 commits into from
Dec 2, 2021
Merged

Use Scons in CI with container #959

merged 38 commits into from
Dec 2, 2021

Conversation

ntindle
Copy link
Member

@ntindle ntindle commented Nov 30, 2021

No description provided.

@ntindle ntindle changed the title Update build.yml Use Scons in CI with container Nov 30, 2021
@ntindle
Copy link
Member Author

ntindle commented Nov 30, 2021

Other than the fact that the container doesn't have cargo it works

@Amaras
Copy link
Member

Amaras commented Nov 30, 2021

Alright, let's try and I'll revert if it doesn't run correctly

@Amaras
Copy link
Member

Amaras commented Nov 30, 2021

@ntindle here is the full problem:
Neither Cargo nor rustc can be found, leading to the build failure (see build 347 374 I think).

The probable cause is as follows:

  1. When the Rust installer runs, it writes the path manipulation inside $HOME/.bashrc
  2. The GH actions run through sh, which doesn't run the ~/.bashrc file.
  3. Pulling the container, we use bash as a shell.

As such, the solution is to do the path manipulation ourselves in the Dockerfile to ensure consistent paths, even with sh

@ntindle ntindle marked this pull request as ready for review December 2, 2021 06:30
@ntindle
Copy link
Member Author

ntindle commented Dec 2, 2021

Okay if whoever merges this doesn't squash and merge, I will cry. THank you for coming to my ted talk

Copy link
Member

@leios leios left a comment

Choose a reason for hiding this comment

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

Ok, this looks fine for me; however, I am not sure how to test this properly.

I am happy to merge though, I trust your judgement

@ntindle
Copy link
Member Author

ntindle commented Dec 2, 2021

To test, read the details of an action and see if scons passes

@ntindle ntindle merged commit ffaa890 into main Dec 2, 2021
@ntindle ntindle deleted the use-container-scons branch December 2, 2021 19:01
@ntindle ntindle mentioned this pull request Dec 4, 2021
4 tasks
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.

4 participants