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.

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

Merge workflows #8045

Merged
merged 47 commits into from
Mar 18, 2022
Merged

Merge workflows #8045

merged 47 commits into from
Mar 18, 2022

Conversation

jneira
Copy link
Member

@jneira jneira commented Mar 14, 2022

  • Continuation of Avoid templating GitHub Actions workflow #7952 and Post remove ci templates #8000

  • Goals:

    • Unify the testing workflows for each os, removing the existing duplication
    • Continue using the validate.sh script as the primary way of running test suites
    • Make the workflow as uniform as possible for all os's, trying to have the same coverage for all of them
    • Try to keep the job matrix as simpler as possible, using defaults or dedicated steps with branch logic
    • Mark workflow as green, marking known broken tests as such, linking the relevant issues
    • comment all possible caveats, corner cases and less obvious rationales
    • Closes Extend new dogfooding ci job to linux and macos #7742
    • Closes Speed up the new CI jobs introduced in #7653 #7655
  • Bonus

    • validate.sh is usable locally in windows, running it in a msys2 shell
    • each workflow run will have the cabal executables attached as artifacts, so you could download it and reproduce errors locally
    • we will be able to replace all validate jobs as required checks to merge with only one job. And we could change them in the future without touching branch protection rules again
  • Continuing the work started in my fork with Merge workflows jneira/cabal#5

  • Caveats

    • You cant restart workflows for os, so a flaky one will make restart the entire workflow
    • We are testing more configurations so times has increased:
    • Also cache size has increased dignificantly, as we have more configurations and we are caching effectively the build dir:
      • Linux 240 Mb * 7 = 1621 Mb
      • Macos 180 Mb * 7 = 1260 Mb
      • Windows 300 Mb * 6 = 1800 Mb
      • ~5 Gb per commit, as github has 10 Gb limit the cache will be invalidated in the third commit of any ongoing pr
        • So the build dir (dist-*) cache will be invalidated very quickly

.gitignore Show resolved Hide resolved
@jneira jneira marked this pull request as ready for review March 14, 2022 14:49
@jneira jneira closed this Mar 14, 2022
@jneira jneira reopened this Mar 14, 2022
@jneira
Copy link
Member Author

jneira commented Mar 14, 2022

CI id not picking the new workflow 🤔

@jneira
Copy link
Member Author

jneira commented Mar 14, 2022

nothing like a rebase to fix all problems, maybe it was confused due to my fork pr

@jneira
Copy link
Member Author

jneira commented Mar 15, 2022

You cant restart workflows for os, so a flaky one will make restart the entire workflow

just I ve seen GitHub has added a rerun jobs from failed, yay!

@jneira
Copy link
Member Author

jneira commented Mar 15, 2022

macOS dogfooding job is failing cause the uploaded tar with the artifact has a folder named GNUSparseFile.0 with the executable inside:

https://github.com/haskell/cabal/suites/5663836477/artifacts/185963938

That did not happen in previous runs nor in my fork, using the same version of the virtual environment:

https://github.com/jneira/cabal/suites/5665936583/artifacts/185923656

weird

EDIT: using --sparse(-S) option did not help: ac6be71

@jneira jneira force-pushed the merge-workflows branch 2 times, most recently from 11700f3 to dc1683b Compare March 16, 2022 17:55
Copy link
Collaborator

@robx robx left a comment

Choose a reason for hiding this comment

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

Generally looks good to me, left a couple minor comments.

No opinion on the general issues (total running time, caching).

.github/workflows/validate.yml Outdated Show resolved Hide resolved
.github/workflows/validate.yml Outdated Show resolved Hide resolved
cabal-install/tests/UnitTests/Distribution/Client/VCS.hs Outdated Show resolved Hide resolved
cabal-testsuite/src/Test/Cabal/Server.hs Outdated Show resolved Hide resolved
@jneira
Copy link
Member Author

jneira commented Mar 17, 2022

the macos cabal executable continue failing randomly with cannot execute binary file, dose not seem related with tar though

maybe related:

@jneira
Copy link
Member Author

jneira commented Mar 18, 2022

ok tar definitely is related, I executed file ./path/to/cabal before tar and it reported cabal: Mach-O 64-bit executable x86_64
After untar the same file it reported cabal: data

@robx
Copy link
Collaborator

robx commented Mar 18, 2022

ok tar definitely is related, I executed file ./path/to/cabal before tar and it reported cabal: Mach-O 64-bit executable x86_64 After untar the same file it reported cabal: data

Just to note that I can confirm this locally: Downloaded the artifact from the bottom here https://github.com/haskell/cabal/actions/runs/2000402197, and it shows the same when I unzip and untar it. Let me know if you'd like me to test something on macOS.

@jneira
Copy link
Member Author

jneira commented Mar 18, 2022

Let me know if you'd like me to test something on macOS.

many thanks for reproduce and report it 🙂

@jneira
Copy link
Member Author

jneira commented Mar 18, 2022

Related: actions/runner-images#2619
Or i hope so, as use gnutar seems to workaround the issue

@Mikolaj
Copy link
Member

Mikolaj commented Mar 18, 2022

Isn't there an option to tar for compatibility with gnutar?

@jneira
Copy link
Member Author

jneira commented Mar 18, 2022

Isn't there an option to tar for compatibility with gnutar?

i've decided to try the other workaround suggested in the issue: call sudo /usr/sbin/purge to clean cache before do the tarball, as it does need any additional installation

@jneira
Copy link
Member Author

jneira commented Mar 18, 2022

PackageTests\InternalLibraries\Haddock\haddock.test.hs is flaky on windows:


<command line>: does not exist: C:\Users\RUNNER~1\AppData\Local\Temp\ghcB217.c


stderr:
*** Exception: Command "D:/a/cabal/cabal/dist-newstyle-validate-ghc-8.6.5/build/x86_64-windows/ghc-8.6.5/cabal-testsuite-3\build\setup\setup" "configure" "-vverbose +markoutput +nowrap" "--distdir" "haddock.dist\work\dist" "--global" "--with-ghc" "C:\ProgramData\chocolatey\lib\ghc.8.6.5\tools\ghc-8.6.5\bin\ghc.exe" "--with-haddock" "C:\ProgramData\chocolatey\lib\ghc.8.6.5\tools\ghc-8.6.5\bin\haddock.exe" "--enable-deterministic" "--prefix=D:\a\cabal\cabal\cabal-testsuite\PackageTests\InternalLibraries\Haddock\haddock.dist\usr" "--package-db=clear" "--package-db=global" "--package-db=C:\sr\ghc-8.6.5\package.db" "--package-db=D:\a\cabal\cabal\dist-newstyle-validate-ghc-8.6.5\packagedb\ghc-8.6.5" "--package-db=D:\a\cabal\cabal\cabal-testsuite\PackageTests\InternalLibraries\Haddock\haddock.dist\packagedb" failed.

We should consider ignore it

@jneira
Copy link
Member Author

jneira commented Mar 18, 2022

Isn't there an option to tar for compatibility with gnutar?

i've decided to try the other workaround suggested in the issue: call sudo /usr/sbin/purge to clean cache before do the tarball, as it does need any additional installation

it did the trick!

@Mikolaj @phadej i will wait to merge some hours just in case you have the opportunity of take a final look
After that we will have to manually merge it and change again required checks and rebase other prs. But it will be the last time, as we will use the final post job as the unique require check for the workflow and it likely will not be changed

Copy link
Member

@Mikolaj Mikolaj left a comment

Choose a reason for hiding this comment

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

Great job.

We should rebrand cabal as CI consultancy and we'd start bringing in profit for HF.

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.

Extend new dogfooding ci job to linux and macos Speed up the new CI jobs introduced in #7653
4 participants