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

CI attempt 2 #156

Merged
merged 18 commits into from
Mar 17, 2023
Merged

CI attempt 2 #156

merged 18 commits into from
Mar 17, 2023

Conversation

michaelpj
Copy link
Contributor

No description provided.

@michaelpj
Copy link
Contributor Author

Okay, this basically works, we just can't actually build plutus-core because of issues with cardano-crypto-class.

@shlevy
Copy link
Contributor

shlevy commented Mar 15, 2023

Can we add a revision to c-c-c to turn off Werror?

@michaelpj
Copy link
Contributor Author

I'm going to - I got diverted into writing a script to add revisions...

@michaelpj michaelpj force-pushed the mpj/builder2 branch 3 times, most recently from 5fc222c to ba7f62f Compare March 16, 2023 12:45
@michaelpj michaelpj requested a review from andreabedini March 16, 2023 16:16
@michaelpj
Copy link
Contributor Author

Okay, I think this is good to go. I would appreciate a review from @andreabedini if he's got time. It will need me to change the name of the required check before we can merge, though.

Maybe also @shlevy might be interested to see how we're using nixbuild.net in the end.

@michaelpj
Copy link
Contributor Author

In a later PR we can work on getting the other packages to work.

Also, a notable restriction I had to make: I was going to build all the versions of the smoke-test packages, but we can't do that because some of them are going to be irredeemably unbuildable with old/new GHCs. I don't know what to do about that. I've compromised on just building the latest versions of things.

Copy link
Contributor

@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.

I hope I didn't go overboard. My intention is to make sure we keep the nix code involved as low-maintainance as possible.

.github/workflows/CI.yml Outdated Show resolved Hide resolved
.github/workflows/CI.yml Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
builder/default.nix Outdated Show resolved Hide resolved
flake.nix Outdated
Comment on lines 81 to 87
let
# The latest version in the set (attrValues sorts by key). Remove the 'recurseForDerivations' here otherwise
# we get that!
latest = versionToValue: lib.last (lib.attrValues (builtins.removeAttrs versionToValue ["recurseForDerivations"]));
compilerPkgs = builtins.getAttr compiler haskellPackages;
smokeTestPkgs = lib.recurseIntoAttrs (lib.genAttrs smokeTestPackages (pkgname: latest (builtins.getAttr pkgname compilerPkgs)));
in smokeTestPkgs);
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel this logic should belong elsewhere. I think it's here because I hid the logic of getting the list of chap package inside ./builder.

I think we should decouple wrangling the CHaP metadata and defining the test jobs.
I think we can make ./builder do only the chap-package-components part:

{ pkgs, CHaP }:
compiler-nix-name: package-name: package-version:
...

Then we move out the part manipulates package.json elsewhere (still in flake.nix? in ./chap.nix? ./builder/chap.nix?)

CHaP:
rec {
  # { "foo" : [ "X.Y.Z" "P.Q.R" ], ... }
  chap-package-attrs = ...,
  
  # { "foo": "X.Y.Z" }
  chap-package-last versions = ... 
}

Then in flake.nix we can just apply the builder to these two attrset.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I did a big refactor, I think this is a lot nicer.

builder/default.nix Show resolved Hide resolved
builder/default.nix Outdated Show resolved Hide resolved
flake.nix Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@michaelpj
Copy link
Contributor Author

Not overboard at all, I like comments :)

@michaelpj
Copy link
Contributor Author

I'm going to merge so we can iterate from here.

@michaelpj michaelpj merged commit 968d3cb into main Mar 17, 2023
@michaelpj michaelpj deleted the mpj/builder2 branch March 17, 2023 12:00
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.

3 participants