Skip to content

Remove tag fetching in docker workflow #35417

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
Merged
merged 1 commit into from
May 22, 2023
Merged

Remove tag fetching in docker workflow #35417

merged 1 commit into from
May 22, 2023

Conversation

tobiasdiez
Copy link
Contributor

@tobiasdiez tobiasdiez commented Apr 2, 2023 •
edited
Loading

📚 Description

It fails in forks (e.g. https://github.com/tobiasdiez/sage/actions/runs/4586854543/jobs/8100123316) and shouldn't be necessary since the tag will be passed anyway via EXTRA_DOCKER_TAGS.

📝 Checklist

  • The title is concise, informative, and self-explanatory.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation accordingly.

⌛ Dependencies

@tobiasdiez tobiasdiez requested a review from mkoeppe April 2, 2023 04:48
@tobiasdiez tobiasdiez mentioned this pull request Apr 2, 2023
Merged
5 tasks
tobiasdiez added a commit to tobiasdiez/sage that referenced this pull request Apr 13, 2023
@tobiasdiez
remove dependency on sagemath#35417
52e226f
mkoeppe
mkoeppe approved these changes Apr 26, 2023
View reviewed changes
Copy link
Contributor

@mkoeppe mkoeppe left a comment

Choose a reason for hiding this comment

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

Indeed, as @dimpase confirms in #35571 (comment)

The "fetch tags" does still have a function, namely to make "git describe" more expressive. But let's get rid of this step at least for now. Thanks

@tobiasdiez
Copy link
Contributor Author

Thanks for the review.

The workflow is still broken btw in forks. The docker upload/download doesn't work, so the next level breaks, e.g. https://github.com/tobiasdiez/sage/actions/runs/4586981626/jobs/8106335802

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 26, 2023

For that failure, see the corresponding minimal-pre workflow (the 1st stage): https://github.com/tobiasdiez/sage/actions/runs/4586981626/jobs/8103083560#step:10:1671

Successfully tagged ghcr.io/tobiasdiez/sage/sage-ubuntu-trusty-toolchain-gcc_9-minimal-configured:aebd2ff067
Successfully tagged ghcr.io/tobiasdiez/sage/sage-ubuntu-trusty-toolchain-gcc_9-minimal-configured:python38
Pushing ghcr.io/tobiasdiez/sage/sage-ubuntu-trusty-toolchain-gcc_9-minimal-configured:aebd2ff067 ghcr.io/tobiasdiez/sage/sage-ubuntu-trusty-toolchain-gcc_9-minimal-configured:python38
The push refers to repository [ghcr.io/tobiasdiez/sage/sage-ubuntu-trusty-toolchain-gcc_9-minimal-configured]
eeb2fe087397: Preparing
238[1673](https://github.com/tobiasdiez/sage/actions/runs/4586981626/jobs/8103083560#step:10:1674)5ef25: Preparing
5f3a7dba7309: Preparing
[...]
30d3c4334a23: Waiting
f2fa9f4cf8fd: Waiting
denied: installation not allowed to Create organization package
(ignoring errors)
The push refers to repository [ghcr.io/tobiasdiez/sage/sage-ubuntu-trusty-toolchain-gcc_9-minimal-configured]

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 26, 2023

We discussed this in #35108 (comment), I'll have to get back to it

dimpase pushed a commit to tobiasdiez/sage that referenced this pull request May 22, 2023
@tobiasdiez @dimpase
remove dependency on sagemath#35417
eed0593
@vbraun vbraun merged commit 2828993 into sagemath:develop May 22, 2023
@tobiasdiez tobiasdiez deleted the tagfetch branch May 23, 2023 01:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
1 more reviewer

@mkoeppe mkoeppe mkoeppe approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
sage-10.1
Development

Successfully merging this pull request may close these issues.
3 participants
@tobiasdiez @mkoeppe @vbraun