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

GitInput: add merge-base support #734

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

GitInput: add merge-base support #734

wants to merge 1 commit into from

Conversation

basvandijk
Copy link
Member

We have been using the following feature since a month at work and it's working well:

This allows fetching the merge-base revision between the specified branch and the specified base branch. This is useful for defining jobs that perform comparisons between the base and the PR.

To enable this specify the base option and point it to the base branch of the PR. For example:

git@github.com:me/my-repo.git my-pr-branch base=master

The plugin will calculate the merge-base revision between my-pr-branch and master, nix-prefetch-git that revision and make it available in the output under the mergeBase attribute like:

{
  outPath = builtins.storePath /nix/store/...-source;
  inputType = "git";
  uri = "git@github.com:me/my-repo.git";
  rev = "<revision of the PR branch>";
  revCount = ...;
  gitTag = "...";
  shortRev = "...";
  mergeBase = {
    outPath = builtins.storePath /nix/store/...-source;
    inputType = "git";
    uri = "git@github.com:me/my-repo.git";
    rev = "<revision of the merge-base of the PR branch and master>";
    revCount = ...;
    gitTag = "...";
    shortRev = "...";
  };
}

@basvandijk
GitInput: add merge-base support
88e0ab6 
This allows fetching the merge-base revision between the specified
branch and the specified base branch. This is useful for defining jobs
that perform comparisons between the base and the PR.

To enable this specify the `base` option and point it to the base
branch of the PR. For example:

```
git@github.com:me/my-repo.git my-pr-branch base=master
```

The plugin will calculate the merge-base revision between
`my-pr-branch` and `master`, `nix-prefetch-git` that revision and make
it available in the output under the `mergeBase` attribute like:

```
{
  outPath = builtins.storePath /nix/store/...-source;
  inputType = "git";
  uri = "git@github.com:me/my-repo.git";
  rev = "<revision of the PR branch>";
  revCount = ...;
  gitTag = "...";
  shortRev = "...";
  mergeBase = {
    outPath = builtins.storePath /nix/store/...-source;
    inputType = "git";
    uri = "git@github.com:me/my-repo.git";
    rev = "<revision of the merge-base of the PR branch and master>";
    revCount = ...;
    gitTag = "...";
    shortRev = "...";
  };
}
```
@basvandijk basvandijk requested a review from edolstra April 8, 2020 14:41
@edolstra
Copy link
Member

edolstra commented Apr 8, 2020 •
edited

Hm, this is a bit problematic for the goal of making Hydra evaluation reproducible for users. For example, with flakes, it's guaranteed that a command like

nix build github:<my-org>/<my-repo>/<rev-hash>#hydraJobs.<job-name>

produces exactly the same result as Hydra. If Hydra passes in special attributes like mergeBase, this is no longer guaranteed.

@basvandijk
Copy link
Member Author

Right but you could say that if the jobset has a mergeBase input the user needs to provide it to get reproducibility. For example if the input is named src he would need to:

nix build github:<my-org>/<my-repo>/<rev-hash>#hydraJobs.<job-name> \
  --arg src 'builtins.fetchGit URI-to-pr // { 
              mergeBase = builtins.fetchGit URI-to-base; 
            }'

The same holds for any other input to the jobset. If a user wants complete reproducibility he needs to provide all required inputs.

@nomeata
Copy link

nomeata commented Apr 9, 2020

Allow me to chime in that this feature made CI qualitatively better for me, who is a developer working on a compiler, becuase it allows what I like to coin differential CI:

Our set of derivations contain one derivation perf that collects metrics (produce code size, other performance numbers). Upon every PR against our compiler, there is now a perf-delta hydra job that fetches the perf from the base merge point (usually some past commit on master) and the PR’s HEAD, produces a report comparing the two, and even comments on the PR with “This does not change any output” or “Code size increased with 5%”. This is super helpful in getting notified about changes, especially unexpected changes, and adds value that cannot really be replicated by CIs that, when testing a PR, treat it like a fully isolated snapshot.

I could image that als nixpkgs could benefit from this (speculating now): It could report things like “this PR changes 25”. This assumes you can enumerate all (or some selected) derivations and write the derivation hashes (without building them, of course) into some output.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@edolstra edolstra Awaiting requested review from edolstra

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@basvandijk @edolstra @nomeata