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

Add a CI step to check significant package output changes resulting from changes to patches.nix #65

Open
sdankel opened this issue May 30, 2023 · 1 comment

Comments

@sdankel
Copy link
Member

sdankel commented May 30, 2023

@mitchmindtree mentioned that nix flake show can tell us which packages would be changed by refresh-manifest.sh.

We could add this to PRs to show this output to help us catch any issues with patch changes.

@mitchmindtree
Copy link
Contributor

Just to clarify, in our chat earlier the nix flake show suggestion was more related to dry-running the difference in package outputs that result from a change to the patches.nix list.

The role of the refresh-manifests.sh script is a little different - namely, it simply produces the manifest files. Changes to patches.nix will never change what manifest files are generated by refresh-manifests.sh. Rather, changes to patches.nix do change the way that manifests are patched and extended before they're used to construct the package outputs.

For addressing the issue we discussed around changes to patches.nix, perhaps a better name for this issue might be something along the lines of Add a CI step to check significant package output changes resulting from changes to patches.nix.

patches.nix changes that would result in breaking builds should be caught by the existing CI, however patches.nix changes that accidentally invalidate large parts of the cache might be trickier to check for (other than spotting CI times increasing) 🤔

One approach might be to have some CI step that runs first (before the builds begin for all platforms) that diffs the output of nix show derivation for each output and checks for significant changes. E.g. if multiple old package derivations have changed, then a patch may have unintentionally changed some old packages and invalidated the cache for older versions.

A similar check could potentially be added to test changes to fliters.nix by diffing the output of nix flake show (which shows a list of all package outputs). This might help to catch cases where a filter was added that accidentally omits loads of older versions by mistake.

@sdankel sdankel changed the title Add a CI step to dry run refresh-manifest Add a CI step to check significant package output changes resulting from changes to patches.nix Apr 19, 2024
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

2 participants