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

Nix: no drun by default, and build with Github Actions #1224

Merged
merged 47 commits into from
Jun 9, 2021

Conversation

nomeata
Copy link
Collaborator

@nomeata nomeata commented Feb 20, 2020

This helps eventual external contributors to build Motoko. The only internal dependency left is drun.

By default, drun is not used. If you have access to the internal repositories, run

touch enable-internals

and things are as before.

This also sets up a Github Action build that builds all stuff, on Linux and Darwin, and uses the ic-hs-test cachix cached (shared with dfinity/ic-hs) to cache build artifacts.

nmattia and others added 8 commits December 11, 2019 16:37
This sets up a simple GitHub Actions workflow that builds all targets on both Linux and Darwin.

The workflow only builds the targets specified in the [DFX Interface](./design/DFX-Interface).
if they are not, warn, but do not try to run the tests, and do not allow
accepting tests.

This means that people who do not have access to `dfinity` can run some
tests (including a public CI like Github Actions).

It also means that `make accept` will not commit `could not find
ic-stub-run` as the expected output, as has happened to us a few times
before.
the reiterates on #1220. The motivation for the refactoring was that the
generated files were not using `gitSource`, so I would get cache misses
and dirty files in the repo. Fixing this was easier if all generated nix
files are in one place, so I did the following changes:

 * All generated nix files are in `nix/generated`. This de-pollutes the
   normal working area with generated files, and keeps them out from
   sources (more precise derivations).
 * The local generated files now use `gitSource`.
 * The generator is now `nix/generate.nix`.
 * I moved complex logic from `nix/default.nix` to `nix/generate.nix`,
   this means everything is in one place, instead of spread over several
   nix files.
 * There is now only a single check to check if the generated files are
   upto date, and a single command to update the files.
 * This check is no longer marked `allowSubstitutes = false`. The check
    isn’t cheap (it requires `cabal2nix`) and we want the check to happen
    on builders, not hydra itself, so that the cache has `cabal2nix`.
 * The check is now included in `all-systems-go`.
@nomeata
Copy link
Collaborator Author

nomeata commented Feb 20, 2020

I guess without a (public) cache running this on GitHub Actions (or external users) is not feasible, as the amount of building happening will likely exceed all limits there?

@nomeata
Copy link
Collaborator Author

nomeata commented Feb 20, 2020

https://github.com/dfinity-lab/motoko/pull/1224/checks?check_run_id=456633738 reports: 22m to build moc-bin without any non-standard cache on Darwin

in #1189 we switched to a fork of nixpkgs to get two patches not in
nixpkgs `master` at that time. Looks like they have made it to
`nixpkgs/master`, so we can track that again (and make the Motoko repo
more self-contained).
better public cache hits
default.nix Outdated Show resolved Hide resolved
mergify bot pushed a commit that referenced this pull request Feb 21, 2020
this removes some redundant logic; cherry-picked from #1224
@nomeata
Copy link
Collaborator Author

nomeata commented May 25, 2021

Thanks to #2540 this now only disables drun, not the rest.

If we can get drun published on dfinity/ci, we’d be completely independent of private repositories!

@nomeata nomeata mentioned this pull request May 29, 2021
5 tasks
@nomeata nomeata changed the title Nix: A flag to disable use of internal repositories Nix: no drun by default, and build with Github Actions Jun 8, 2021
@nomeata nomeata marked this pull request as ready for review June 8, 2021 07:29
@nomeata nomeata requested review from crusso and kritzcreek June 8, 2021 07:30
- uses: cachix/cachix-action@v10
with:
name: ic-hs-test
authToken: '${{ secrets.CACHIX_AUTH_TOKEN }}'
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm assuming you're the one holding this secret at the moment. Who else should have a copy?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is currently a cache on my personal cachix account. I suggest we run with that until we are open and things work, then we can migrate to a cache owned by some organizational account (but I fear the confusion if we try to solve that problem at the same time).

Using a different cache is rather simple and not very disruptive.

Copy link
Contributor

Choose a reason for hiding this comment

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

Understood. Let's just make sure we don't forget about this. "Nichts hält so lange wie ein Provisorium"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think the idiom goes “wie ein gutes Provisorium” :-)

.github/workflows/test.yml Outdated Show resolved Hide resolved
Building.md Outdated Show resolved Hide resolved
curl -L https://nixos.org/nix/install | sh
```

You should also enable a nix cache to get all dependencies pre-built.
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we (the internal users) also need to run these steps now?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No. Or rather, not yet. It doesn’t hurt though (you might get faster downloads than over VPN?)

Building.md Outdated Show resolved Hide resolved
nomeata and others added 2 commits June 8, 2021 09:49
Co-authored-by: Christoph Hegemann <6189397+kritzcreek@users.noreply.github.com>
@nomeata nomeata added the automerge-squash When ready, merge (using squash) label Jun 8, 2021
external contributors cannot run it.

The build system is setup by default to _not_ load the `drun` tool. If you have
access to this tool, create an empty file called `enable-internals` in this repository
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this file should not be added to git right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right, that’s why it’s in .gitignore, so you’d have to try hard to add it. And it would break CI, so we’d notice.

@nomeata
Copy link
Collaborator Author

nomeata commented Jun 8, 2021

Shall we merge this?

@osa1
Copy link
Contributor

osa1 commented Jun 9, 2021

@nomeata since this doesn't add an enable-internals file to the repo, doesn't this disable testing with drun in the main repo?

@osa1
Copy link
Contributor

osa1 commented Jun 9, 2021

This helps eventual external contributors to build Motoko

Why would external contributors need drun to build Motoko? It's used for testing, not building, right?

@nomeata
Copy link
Collaborator Author

nomeata commented Jun 9, 2021

@nomeata since this doesn't add an enable-internals file to the repo, doesn't this disable testing with drun in the main repo?

Developers with internal access should touch the file locally. The internal CI sets internal = true

@nomeata
Copy link
Collaborator Author

nomeata commented Jun 9, 2021

This helps eventual external contributors to build Motoko

Why would external contributors need drun to build Motoko? It's used for testing, not building, right?

Yes, but external contributors should also be able to run the test suite, at least as far as possible

@nomeata nomeata merged commit 399b8e8 into master Jun 9, 2021
@nomeata nomeata deleted the joachim/public-build branch June 9, 2021 09:33
@mergify mergify bot removed the automerge-squash When ready, merge (using squash) label Jun 9, 2021
@nomeata nomeata mentioned this pull request Jun 9, 2021
8 tasks
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.

7 participants