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

Migrate release CI back to github #9437

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Migrate release CI back to github #9437

wants to merge 2 commits into from

Conversation

hasufell
Copy link
Member

No description provided.

@hasufell hasufell marked this pull request as draft November 13, 2023 09:43
@hasufell hasufell force-pushed the purge-gitlab branch 16 times, most recently from 001d0e1 to bb01a96 Compare November 13, 2023 14:57
@hasufell
Copy link
Member Author

Also includes:

  • script to download artifacts
  • script to create yaml snippet for ghcup

@hasufell hasufell force-pushed the purge-gitlab branch 2 times, most recently from 5c37718 to 34a7c9c Compare November 13, 2023 15:45
@hasufell hasufell marked this pull request as ready for review November 13, 2023 15:45
@hasufell hasufell force-pushed the purge-gitlab branch 2 times, most recently from 3187a1a to 8781d59 Compare November 13, 2023 15:53
@hasufell
Copy link
Member Author

Workflow:

  • push tag cabal-install-v4.0.0.0
  • wait for github and cirrus builds to finish
  • run scripts/release/download-gh-artifacts.sh cabal-install-v4.0.0.0 <your-gpg-email>
  • run scripts/release/create-yaml-snippet.sh cabal-install-v4.0.0.0
    • based on the output create a ghcup-metadata PR

Kleidukos
Kleidukos previously approved these changes Nov 13, 2023
Copy link
Member

@Kleidukos Kleidukos left a comment

Choose a reason for hiding this comment

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

From my perspective this is quite appealing and I would like to give it a try for the next release.

@chreekat
Copy link
Collaborator

chreekat commented Nov 15, 2023

Time comparison:

Gitlab CI: 98m 15s
    https://gitlab.haskell.org/haskell/cabal/-/pipelines/86388
GitHub CI: 32m 35s
    https://github.com/haskell/cabal/actions/runs/6851573157

Ironically, the GitLab pipeline took forever because the Darwin job had to queue
for an hour for an open spot. The GitHub job runs on the same server and
therefore competes for the same resource. It just gets to jump the queue since
the two job runners don't know about each other.

@hasufell
Copy link
Member Author

I do think the GitLab stuff should be kept as a backup as part of good change management, however, so I will request that change.

That would mean we just increased complexity, which defeats the purpose of this PR.

If anything goes extremely wrong during the next release, you can restore the gitlab code from the git history.

@chreekat
Copy link
Collaborator

That would mean we just increased complexity, which defeats the purpose of this PR.

If it wasn't clear, I meant to keep it around for the next release and then dropping it. I hope that doesn't defeat the purpose. This change is bigger than just reimplementing some code ,and keeping two systems running in parallel is good practice. And basically free in this case.

@Mikolaj
Copy link
Member

Mikolaj commented Nov 15, 2023

keep it around for the next release and then dropping it

Regarding this one point, I'd make it two releases and I'd keep the gitlab CI running all the time to ensure we spot problems in it ASAP, just as we do today.

While I'm talking, @hasufell, thank you for the PR. I remember the big appeal of your proposal long ago, now materialised, was that we'd test what we release. Does this PR bring us closer to the point where we test daily the same binaries that will be released? In particular, are our normal GHA tests run on these artifacts, and when not applicable, on the exact commits from which the artifacts are created? What's the roadmap to unify CI and release CI, to a reasonable extent?

@hasufell
Copy link
Member Author

That would mean we just increased complexity, which defeats the purpose of this PR.

If it wasn't clear, I meant to keep it around for the next release and then dropping it. I hope that doesn't defeat the purpose. This change is bigger than just reimplementing some code ,and keeping two systems running in parallel is good practice.

I don't agree with this at all. There's no indication this is necessary. In HLS we also didn't need it and it has a much more complex CI.

@chreekat
Copy link
Collaborator

That would mean we just increased complexity, which defeats the purpose of this PR.

If it wasn't clear, I meant to keep it around for the next release and then dropping it. I hope that doesn't defeat the purpose. This change is bigger than just reimplementing some code ,and keeping two systems running in parallel is good practice.

I don't agree with this at all. There's no indication this is necessary. In HLS we also didn't need it and it has a much more complex CI.

To avoid a deadlock I've created https://github.com/chreekat/cabal/tree/b/add-github-release-ci, which is an edited version of this branch that keeps GitLab CI. I hope it won't be necessary.

@hasufell
Copy link
Member Author

I can split the PR in two commits, so that Gitlab removal is easier to revert in case it's necessary.

But I won't subscribe to "let's keep code in master, just in case". That's bad engineering practice.

@Mikolaj
Copy link
Member

Mikolaj commented Nov 16, 2023

But I won't subscribe to "let's keep code in master, just in case". That's bad engineering practice.

I don't think anybody is proposing that. Gitlab CI would be running continuously so that we can spot any problems and fix them ASAP and during a release, if needed, the release manager would be able to compare the results of gitlab and github, test behaviour of the artifacts from both systems, etc.

@hasufell
Copy link
Member Author

I have decided to build bindists myself for the next cabal release, so cabal's CI isn't relevant for GHCup directly anymore.

You can use this PR and the contained patch as you please.

@hasufell hasufell closed this Nov 16, 2023
@andreabedini andreabedini reopened this Nov 17, 2023
@andreabedini
Copy link
Collaborator

I have reopened this to prevent us losing valuable work.

@andreabedini
Copy link
Collaborator

And thank you @hasufell for working on this!

Copy link
Collaborator

@andreabedini andreabedini left a comment

Choose a reason for hiding this comment

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

Just leaving some comments as I had the time to read this properly. It looks quite comprehensive! good stuff.

PS: @hasufell you have been clear that you don't intend to continue working on this so my comments are not to be taken as request for more work from you :-) They more like notes to my future self.


# https://github.com/haskell/cabal/issues/7313#issuecomment-811851884
if [ "$(getconf LONG_BIT)" == "32" ] || [ "${DISTRO}" == "CentOS" ] ; then
echo 'constraints: lukko -ofd-locking' >> cabal.project.release.local
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe this should just be included in cabal.project.release under a if arch(I386) conditional. I am not sure if there is something specific to CentOS, it does not seem to be mentioned in the thread above

args=(
-w "ghc-$GHC_VERSION"
--disable-profiling
--enable-executable-stripping
Copy link
Collaborator

Choose a reason for hiding this comment

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

other things that should be in cabal.project.release

timeout_in: 120m
only_if: $CIRRUS_TAG != ''
env:
ADD_CABAL_ARGS: "--enable-split-sections"
Copy link
Collaborator

Choose a reason for hiding this comment

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

this can be implemented in cabal.project.release under a conditional if os(FreeBSD)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why would this be conditional on platform? AFAIK it's only Windows that doesn't support it, and it's a win everywhere else. (Macs ignore the option since they always do dead code stripping, although GHC needed a lot of hackery to make use of it safely, but that's not our problem.)

uses: actions/checkout@v3

- if: matrix.ARCH == 'ARM'
uses: docker://hasufell/arm32v7-ubuntu-haskell:focal
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are there officially maintaned images we can use?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I would expect that there are official Ubuntu images, but they don't have any Haskell stuff preinstalled and if they did it'd be Ubuntu's ancient versions.

Copy link
Member Author

@hasufell hasufell Sep 19, 2024

Choose a reason for hiding this comment

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

The images are autorebuilt: https://github.com/haskell/ghcup-hs/actions/workflows/docker.yaml

However, you should be using docker://hasufell/arm32v7-debian-haskell:10.

See my latest action: https://github.com/stable-haskell/cabal/blob/cabal-install-v3.12.1.0/.github/workflows/release.yaml

ARTIFACT: ${{ matrix.ARTIFACT }}

- if: matrix.ARCH == 'ARM64'
uses: docker://hasufell/arm64v8-ubuntu-haskell:focal
Copy link
Collaborator

Choose a reason for hiding this comment

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

same as above


- name: git config
run: |
git config --global --get-all safe.directory | grep '^\*$' || git config --global --add safe.directory "*"
Copy link
Collaborator

Choose a reason for hiding this comment

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

in the current ci we just do

git config --global protocol.file.allow always

and we have a comment point to https://bugs.launchpad.net/ubuntu/+source/git/+bug/1993586 (cabal PR #8546)

@geekosaur
Copy link
Collaborator

I'd be willing to take this on after we get 3.12.1.0 out the door.

@andreabedini
Copy link
Collaborator

I'd be willing to take this on after we get 3.12.1.0 out the door.

Happy to help too.

@geekosaur
Copy link
Collaborator

geekosaur commented Sep 18, 2024

I have temporarily reinstated the Gitlab CI stuff until we come to an agreement about it. (I'm on the side that it's always possible to arrange things so it can be reverted back in if we need it. I mean, that's what git is for, isn't it?)

(And part of the reason I did that is so I can take it out in a distinct commit that can easily be reverted.)

@geekosaur geekosaur force-pushed the purge-gitlab branch 2 times, most recently from 90334dd to 3f2515c Compare September 19, 2024 08:54
@geekosaur
Copy link
Collaborator

Manually squashed, added Authored-by: to keep original attribution.

@geekosaur
Copy link
Collaborator

geekosaur commented Sep 19, 2024

Gitlab CI removed again, revert a12c6be to bring it back if needed.

Authored-by: Julian Ospald (@hasufell)
Revert this commit at need.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants