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

Determine regressions from target branch #85

Open
jonringer opened this issue Feb 17, 2020 · 16 comments
Open

Determine regressions from target branch #85

jonringer opened this issue Feb 17, 2020 · 16 comments

Comments

@jonringer
Copy link
Contributor

I would be more than okay with building the target branch's packages to see if any regression happened as part of a nixpkgs-review. Something to the affect of New successes, new failures, still failing would be nice. Similar to hydra evaluations.

My current workflow is to take all the packages that failed and run nix build -f . --keep-going $@. And then manually see which ones passed. This isn't too bad if it's just <10 failures, but some reviews (especially python packages), can have 20-60 failures, and it becomes very difficult to determine regressions.

@Mic92
Copy link
Owner

Mic92 commented Feb 17, 2020

So you mean building before and after? Yeah. That would be nice to have.

@jonringer
Copy link
Contributor Author

Yes, essentially a "package diff" which told you what changed.

@Mic92
Copy link
Owner

Mic92 commented Feb 18, 2020

That information can be cheaply obtained from hydra, in case hydra has attempted to build the old package before.

@jonringer
Copy link
Contributor Author

Yea, but I don't really mind brute forcing with more CPU. Would you be willing to work with me if I created a PR that did create the package diffs?

@jonringer
Copy link
Contributor Author

Also when targeting master, you're likely to get cached hits if they weren't broken already

@Mic92
Copy link
Owner

Mic92 commented Feb 19, 2020

Yea, but I don't really mind brute forcing with more CPU. Would you be willing to work with me if I created a PR that did create the package diffs?

Yes.

@Mic92
Copy link
Owner

Mic92 commented Feb 19, 2020

Also when targeting master, you're likely to get cached hits if they weren't broken already

Yes. It is just an optimization, but there is no need to actually download it or to rebuild an already failed build.

@jonringer
Copy link
Contributor Author

maybe when the 20.03 hype has died down I'll get around to this, going to focus on nixpkgs during the release cycle though.

Yes. It is just an optimization, but there is no need to actually download it or to rebuild an already failed build.

I think it would be useful to determine that a regression could have happened.

@jonringer
Copy link
Contributor Author

actually, maybe it would be good to take a break from constant PR reviews

@jonringer
Copy link
Contributor Author

for the switch, are you good with -d, --diff?

e.g.
nix-review pr XXXX --diff?

@Mic92
Copy link
Owner

Mic92 commented Feb 20, 2020

for the switch, are you good with -d, --diff?

e.g.
nix-review pr XXXX --diff?

Mhm. I also have don't have a good name yet, but if I would see the --diff flag I would not know what it does.

@jonringer
Copy link
Contributor Author

--delta would also work, but it's a little "math-y". --package-diff --set-diff

maybe --changes?
--regressions?

@Mic92
Copy link
Owner

Mic92 commented Feb 24, 2020

--regressions seem to be the best most expressive one because nix-review is already testing changes, but not the packages before they were changed.

@jonringer
Copy link
Contributor Author

I just noticed this, I think it's a nice touch:

$ nix-env -f /home/jon/.cache/nixpkgs-review/pr-80755/nixpkgs -qaP --xml --out-path --show-trace --meta
21 package updated:
bcc errbot libselinux mba6x_bl nixpart php-couchbase php-couchbase php-pcs php-pcs php-php_excel php-php_excel php-php_excel php-php_excel php-php_excel php-php_excel poezio pyblock python-notify python-notify python3.7-python-slackclient (1.2.1 → 2.5.0) python3.8-python-slackclient (1.2.1 → 2.5.0)

1 package removed:
python2.7-python-slackclient (†2.5.0)

@Mic92
Copy link
Owner

Mic92 commented Mar 1, 2020

This only works for local evaluation right now, though.

@timokau
Copy link

timokau commented Apr 16, 2020

Also when targeting master, you're likely to get cached hits if they weren't broken already

Yes. It is just an optimization, but there is no need to actually download it or to rebuild an already failed build.

For what its worth, that is one of the things I try to do with nix-build-status (part of nix-bisect). Not sure if that can or should be reused here.

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

No branches or pull requests

3 participants