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.

Sign up for GitHub

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

Jump to bottom

Use Ubuntu 24.04 for rename from util-linux #1856

Merged
merged 1 commit into from
Nov 19, 2024
Merged

Use Ubuntu 24.04 for rename from util-linux #1856

merged 1 commit into from
Nov 19, 2024

Conversation

ekohl
Copy link
Collaborator

@ekohl ekohl commented Nov 19, 2024 •
edited
Loading

I'm unsure why this broke. I also looked at https://honkit.netlify.app/ebook which mentions you can publish an ebook with a specific name, but figured this was the shortest path. It's untested now.

Right after submitting this I had a theory why it broke. On the first run it passed, but after that it started to fail. The first run it couldn't have cached the package with awalsh128/cache-apt-pkgs-action while it has on the later runs.

/usr/bin/rename is provided via alternatives:

Setting up rename (2.02-1) ...
update-alternatives: using /usr/bin/file-rename to provide /usr/bin/rename (rename) in auto mode
update-alternatives: warning: skip creation of /usr/share/man/man1/rename.1.gz because associated file /us
r/share/man/man1/file-rename.1p.gz (of link group rename) doesn't exist

I suspect awalsh128/cache-apt-pkgs-action ignores alternatives so /usr/bin/rename isn't present when a cache is used.

This updates to Ubuntu 24.04 and relies on rename from util-linux which bypasses the need to cache it altogether.

I can't think of an easy way to test this.

Fixes #1855

@ekohl ekohl requested a review from a team as a code owner November 19, 2024 12:29
@ekohl ekohl marked this pull request as draft November 19, 2024 12:33
@ekohl
Run GitHub Actions on Ubuntu 24.04
584056c 
On Ubuntu the file /usr/bin/rename is managed through alternatives
because there are multiple implementations. It appears that
awalsh128/cache-apt-pkgs-action doesn't update alternatives when a
package is cached which means /usr/bin/rename isn't present.

The latest image will change to 24.04 on between December and January
but on 24.04 there is rename.ul from util-linux which also provides the
rename. That avoids the need to rely on awalsh128/cache-apt-pkgs-action
for it and the alternatives interaction.
@ekohl ekohl force-pushed the fix-rename-in-deploy branch from 4bbce89 to 584056c Compare November 19, 2024 12:38
@ekohl ekohl marked this pull request as ready for review November 19, 2024 12:41
@ekohl ekohl mentioned this pull request Nov 19, 2024
Closed
@ekohl ekohl changed the title Use rename.ul from Ubuntu 24.04 in deploy flow Use Ubuntu 24.04 for rename from util-linux Nov 19, 2024
@das-g
Copy link
Member

das-g commented Nov 19, 2024 •
edited
Loading

I can't think of an easy way to test this.

Can we make the stuff that runs for PRs (currently defined in .github/workflows/test.yml) more similar to that that would run on master (currently defined in .github/workflows/deploy.yml)? IMO the only difference should be that PRs must not deploy to the production location.

That way, the checks for PRs would test almost everything of the productive CI/CD (e.g. also the renaming).

das-g
das-g approved these changes Nov 19, 2024
View reviewed changes
Copy link
Member

@das-g das-g left a comment

Choose a reason for hiding this comment

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

Seems reasonable.

@das-g
Copy link
Member

das-g commented Nov 19, 2024

I can't think of an easy way to test this.

Can we make the stuff that runs for PRs (currently defined in .github/workflows/test.yml) more similar to that that would run on master (currently defined in .github/workflows/deploy.yml)? IMO the only difference should be that PRs shouldn't deploy to the production location.

That way, the checks for PRs would test almost everything of the productive CI/CD (e.g. also the renaming).

Though for now / for this change here: As the continuous build is currently broken anyway, we could just merge this and see if it gets better. I can't imagine it to have any disastrous consequences: Either it makes it work, or it keeps the build failing similarly to now. 🤷

Thus: Feel free to merge. :shipit:

@ekohl
Copy link
Collaborator Author

ekohl commented Nov 19, 2024

Can we make the stuff that runs for PRs (currently defined in .github/workflows/test.yml) more similar to that that would run on master (currently defined in .github/workflows/deploy.yml)? IMO the only difference should be that PRs must not deploy to the production location.

#1835 was the start for that, but I rarely have the time to work on Django things. That one was mostly done during a DjangoGirls workshop where the students didn't need a lot of guidance.

@ekohl ekohl merged commit ef2ca84 into DjangoGirls:master Nov 19, 2024
1 check passed
@ekohl ekohl deleted the fix-rename-in-deploy branch November 19, 2024 16:27
@ekohl
Copy link
Collaborator Author

ekohl commented Nov 25, 2024

This is really weird: https://github.com/DjangoGirls/tutorial/actions/runs/11917391625/job/33212329587 failed on

/home/runner/work/_temp/0bb45bc1-4c28-4c5b-a117-ee1a9563fd3c.sh: line 4: rename: command not found

But then later versions did deploy correctly. Almost like it was always using the old workflow file. Perhaps that is why the original change only started to break after the first run?

This is not how I thought GHA was supposed to work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@das-g das-g das-g approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

The build in the CI is broken
2 participants
@ekohl @das-g