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

CI Linux: Fix "optional", "experimental" jobs #36668

Merged
merged 1 commit into from
Nov 10, 2023
Merged

CI Linux: Fix "optional", "experimental" jobs #36668

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/workflows/docker.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,9 @@ jobs:
echo "DOCKER_PUSH_REPOSITORY=$(echo ${{ inputs.docker_push_repository }} | tr "[:upper:]" "[:lower:]")" >> $GITHUB_ENV
echo "DOCKER_CONFIG_FILE=$HOME/.docker/config.json" >> $GITHUB_ENV
fi

- name: Determine Docker tags to use
run: |
# This line needs to be run before the step "Merge CI fixes from sagemath/sage".
Copy link
Collaborator

Choose a reason for hiding this comment

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

This fix seems to indicate that the previous step (logging to ghcr.io) is not really necessary (used) for subsequent steps. So perhaps the step is here to provide the environment variables to the calling workflow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The environment variables are set for the following steps in this job, specifically for invoking tox.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I meant

(A): logging into docker and setting the environment variables DOCKER_PUSH_REPOSITORY and DOCKER_CONFIG_FILE

in the previous step to the "Determine Docker tags to use" step.

If the idea is to run "Determine Docker tags to use" step even when inputs.docker_push_repository is empty, then it means that (A) is not necessary for the steps following and including the "Determine Docker tags to use" step.

So I thought (A) could be moved out to the calling workflow and don't have to be in the docker workflow. I am not suggesting we should do it. I just want to know if I understand the situation correctly. Am I right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the idea is to run "Determine Docker tags to use" step even when inputs.docker_push_repository is empty, then it means that (A) is not necessary for the steps following and including the "Determine Docker tags to use" step.

That's right, the two parts now separated do not interact with each other. Each part sets some environment variables for the following steps.

So I thought (A) could be moved out to the calling workflow and don't have to be in the docker workflow. I am not suggesting we should do it. I just want to know if I understand the situation correctly. Am I right?

No; environment variables set by appending to the file $GITHUB_ENV only take effect for the next steps of a job. But a reusable workflow is run as a job, not a step of a job. See https://docs.github.com/en/actions/using-workflows/reusing-workflows#limitations

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK. Thanks.

DOCKER_TAG="$(git describe --dirty --always)"
echo "DOCKER_TAG=$DOCKER_TAG" >> $GITHUB_ENV
Expand Down
Loading