Skip to content

GitHub Action Ubuntu job doesn't check if generated files are up to date #380

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

Closed
vstinner opened this issue Oct 26, 2020 · 9 comments
Closed

Comments

@vstinner
Copy link
Member

Previously, Travis CI was mandatory on Python PRs. Travis CI was made optional, and instead GHA Ubuntu was made required.

The problem is that only Travis CI checked if generated files are up to date:

  - eval "$(pyenv init -)"
  - pyenv global 3.8
  - PYTHON_FOR_REGEN=python3.8 make -j4 regen-all
  - changes=`git status --porcelain`
  - |
      # Check for changes in regenerated files
      if ! test -z "$changes"
      then
        echo "Generated files not up to date"
        echo "$changes"
        exit 1
      fi
  - make -j4
  - make pythoninfo

These commands should be copied and adapted in .github/workflows/build.yml.

Is there a volunteer to do that?

cc @FFY00 @ammaraskar

@FFY00
Copy link
Member

FFY00 commented Oct 27, 2020

I can look at this tomorrow 😊

@brettcannon
Copy link
Member

I actually have plans to eventually make this a separate GitHub Action from the build workflows, but who the heck knows when I will have time to make that actually happen. 😄

@vstinner
Copy link
Member Author

I actually have plans to eventually make this a separate GitHub Action from the build workflows, but who the heck knows when I will have time to make that actually happen. smile

I agree that it would be more reliable to not run "make regen-all" for the regular build. It's a good idea to have a separated job. It would also helper contributors to understand a job failure. Previous, the Travis CI error was not obvious.

@vstinner
Copy link
Member Author

Oh, by the way, Travis CI also checks for "make smelly": ensure that all symbols exported by libpython start with "Py" or "_Py".

@FFY00
Copy link
Member

FFY00 commented Oct 30, 2020

@vstinner should this be part of build actions for every OS, or is it fine to just run it one time on one OS (in a separate action)?

@vstinner
Copy link
Member Author

It's enough to test it on a single OS. I suggest to use Linux, since the checks were run on Linux previously.

A separated action would be better.

@vstinner
Copy link
Member Author

FFY00 added a commit to FFY00/cpython that referenced this issue Nov 19, 2020
https: //github.com/python/core-workflow/issues/380
Signed-off-by: Filipe Laíns <lains@archlinux.org>
vstinner pushed a commit to python/cpython that referenced this issue Nov 20, 2020
…H-23042)

See https: //github.com/python/core-workflow/issues/380

Signed-off-by: Filipe Laíns <lains@archlinux.org>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Nov 24, 2020
…ythonGH-23042)

See https: //github.com/python/core-workflow/issues/380

Signed-off-by: Filipe Laíns <lains@archlinux.org>
(cherry picked from commit d20b7ed)

Co-authored-by: Filipe Laíns <lains@archlinux.org>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Nov 24, 2020
…ythonGH-23042)

See https: //github.com/python/core-workflow/issues/380

Signed-off-by: Filipe Laíns <lains@archlinux.org>
(cherry picked from commit d20b7ed)

Co-authored-by: Filipe Laíns <lains@archlinux.org>
miss-islington added a commit to python/cpython that referenced this issue Nov 24, 2020
…H-23042)

See https: //github.com/python/core-workflow/issues/380

Signed-off-by: Filipe Laíns <lains@archlinux.org>
(cherry picked from commit d20b7ed)

Co-authored-by: Filipe Laíns <lains@archlinux.org>
miss-islington added a commit to python/cpython that referenced this issue Nov 24, 2020
…H-23042)

See https: //github.com/python/core-workflow/issues/380

Signed-off-by: Filipe Laíns <lains@archlinux.org>
(cherry picked from commit d20b7ed)

Co-authored-by: Filipe Laíns <lains@archlinux.org>
adorilson pushed a commit to adorilson/cpython that referenced this issue Mar 13, 2021
@nightlark
Copy link

nightlark commented Aug 21, 2021

Is there anything left to do for this issue? It looks like https://bugs.python.org/issue42212 on the python bug tracker is already marked as closed.

@brettcannon what did you have in mind for turning this into a separate GitHub Action? Is that still something you want done?

@brettcannon
Copy link
Member

@nightlark I created https://github.com/brettcannon/check-for-changed-files, but there's already something being done using make patchcheck so it doesn't seem necessary.

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

No branches or pull requests

4 participants