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

Update to latest haskell.nix #1106

Merged
merged 5 commits into from
Jan 9, 2020
Merged

Update to latest haskell.nix #1106

merged 5 commits into from
Jan 9, 2020

Conversation

hamishmack
Copy link
Contributor

@hamishmack hamishmack commented Dec 2, 2019 •
edited by rvl
Loading

This PR is to prepare for updating to latest haskell.nix. It is likely we will need #1100 and #1101 to get the tests to pass.

Hydra jobs - See under x86_64-pc-mingw32.checks for tests running under wine.

  • x86_64-pc-mingw32.checks.bech32.bech32-test.x86_64-linux - pass
  • x86_64-pc-mingw32.checks.cardano-wallet-cli.unit.x86_64-linux - pass
  • x86_64-pc-mingw32.checks.cardano-wallet-core.unit.x86_64-linux - pass
  • x86_64-pc-mingw32.checks.cardano-wallet-jormungandr.integration.x86_64-linux - failed
  • x86_64-pc-mingw32.checks.cardano-wallet-jormungandr.unit.x86_64-linux - pass
  • x86_64-pc-mingw32.checks.cardano-wallet-launcher.unit.x86_64-linux - failed
  • x86_64-pc-mingw32.checks.text-class.unit.x86_64-linux - pass

@hamishmack hamishmack requested a review from rvl December 2, 2019 23:27
@hamishmack hamishmack self-assigned this Dec 2, 2019
@hamishmack hamishmack force-pushed the hkm/haskell-nix-update branch from 971817e to bf51372 Compare December 3, 2019 04:42
@rvl
Copy link
Contributor

rvl commented Dec 3, 2019 •
edited
Loading

Once this PR is complete we should have the Windows tests running under Wine. Thanks @hamishmack

Hydra jobset

@hamishmack
Copy link
Contributor Author

I'm working on a fix for haskell.nix to make makeRelativeToProject work with haskellLib.check. The problem is that makeRelativeToProject captures the temporary dir used while building the test component and by the time the check derivation runs that directory is gone.

I'm working on a change that will makes put the component source (with patches applied into the store) and build from there). Failing that we will need to use an environment variable to override the location of getDataDir. I'm hoping the fix for makeRelativeToProject will work as it would be nice to be able to safely use that with haskell.nix.

@rvl
Copy link
Contributor

rvl commented Dec 3, 2019

OK I see. If the universal fix for makeRelativeToProject doesn't work, then Test.Utils.Paths.getTestData can be modified to use ./test/data when built under nix.

@rvl
Copy link
Contributor

rvl commented Dec 3, 2019

See #1109 for a fix for the test data directory.

@hamishmack hamishmack force-pushed the hkm/haskell-nix-update branch 3 times, most recently from 9463c47 to 9515a76 Compare December 10, 2019 04:33
@hamishmack hamishmack force-pushed the hkm/haskell-nix-update branch from 68cf491 to 15a008a Compare December 13, 2019 03:50
rvl
rvl suggested changes Dec 19, 2019
View reviewed changes
Copy link
Contributor

@rvl rvl left a comment

Choose a reason for hiding this comment

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

Looks good @hamishmack , and I see that the tests are now being run under wine.

Is there anything left to fix? Because this PR is still draft.

Could you please put the stylish-haskell package change and the release.nix memoization in separate PRs where they can be reviewed separately?

release.nix Outdated
gitrev = cardano-wallet.rev;
};

with pkgs.lib;

let
testsSupportedSystems = [ "x86_64-linux" "x86_64-darwin" ];
collectTests = ds: filter (d: elem d.system testsSupportedSystems) (collect isDerivation ds);
# Adds the package name to the test derivations for windows-testing-bundle.nix
# (passthru.identity.name does not seem to survive release-lib.nix)
Copy link
Contributor

Choose a reason for hiding this comment

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

If passthru.identity.name gets removed, then I think it would be just be simplest to remove this change and the corresponding one in windows-testing-bundle.nix.

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 problem is that $pkg is no longer the packager name. Tests and benchmarks are now in bin subdir (not a package id based subdir). I'll replace $pkg with "bin" to make this clear.

We need to ensure that the executable in the windows bundle has a unique location (otherwise tests like unit-tests will clash when we copy them to the bundle). We could zip [1..] with the list and put each executable in a numbered sub dir, but I think it might be more user friendly to keep using the package name somehow.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK got it. I have split the function and added an extra comment.

default.nix Outdated Show resolved Hide resolved
nix/haskell-nix-src.json Outdated Show resolved Hide resolved
@hamishmack hamishmack marked this pull request as ready for review January 7, 2020 23:15
@hamishmack hamishmack requested a review from rvl January 7, 2020 23:16
@rvl rvl force-pushed the hkm/haskell-nix-update branch from 65726ff to 8c93e0d Compare January 9, 2020 06:22
@rvl rvl force-pushed the hkm/haskell-nix-update branch from 8c93e0d to aa6c57c Compare January 9, 2020 06:35
@rvl
Copy link
Contributor

rvl commented Jan 9, 2020

I have rebased and moved the stylish-haskell package change and the release.nix memoization into #1120.

@rvl rvl force-pushed the hkm/haskell-nix-update branch from aa6c57c to a46c0f9 Compare January 9, 2020 07:08
@rvl
Copy link
Contributor

rvl commented Jan 9, 2020

I would feel inclined to merge this now and fix the two test suite failures in a subsequent PR.

@piotr-iohk piotr-iohk mentioned this pull request Jan 9, 2020
Closed
@KtorZ
Copy link
Member

KtorZ commented Jan 9, 2020

bors r+

iohk-bors bot added a commit that referenced this pull request Jan 9, 2020
@iohk-bors @hamishmack @rvl
Merge #1106
944ecdb 
1106: Update to latest haskell.nix r=KtorZ a=hamishmack

This PR is to prepare for updating to latest haskell.nix.  It is likely we will need #1100 and #1101 to get the tests to pass.

[Hydra jobs](https://hydra.iohk.io/jobset/Cardano/cardano-wallet-pr-1106#tabs-jobs) - See under `x86_64-pc-mingw32.checks` for tests running under wine.

- x86_64-pc-mingw32.checks.bech32.bech32-test.x86_64-linux - pass
- x86_64-pc-mingw32.checks.cardano-wallet-cli.unit.x86_64-linux - pass
- x86_64-pc-mingw32.checks.cardano-wallet-core.unit.x86_64-linux - pass
- x86_64-pc-mingw32.checks.cardano-wallet-jormungandr.integration.x86_64-linux - [failed](https://hydra.iohk.io/build/1550317/nixlog/1)
- x86_64-pc-mingw32.checks.cardano-wallet-jormungandr.unit.x86_64-linux - pass	
- x86_64-pc-mingw32.checks.cardano-wallet-launcher.unit.x86_64-linux - [failed](https://hydra.iohk.io/build/1550326/nixlog/2)
- x86_64-pc-mingw32.checks.text-class.unit.x86_64-linux - pass


Co-authored-by: Hamish Mackenzie <[email protected]>
Co-authored-by: Rodney Lorrimar <[email protected]>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jan 9, 2020

Build succeeded

@iohk-bors iohk-bors bot merged commit a46c0f9 into master Jan 9, 2020
@KtorZ KtorZ deleted the hkm/haskell-nix-update branch January 9, 2020 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rvl rvl rvl approved these changes

Assignees

@hamishmack hamishmack

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@hamishmack @rvl @KtorZ