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

Jump to bottom

crun: 0.10.6 -> 0.11 #77483

Merged
merged 1 commit into from Jan 23, 2020
Merged

crun: 0.10.6 -> 0.11 #77483

merged 1 commit into from Jan 23, 2020

Conversation

zowoq
Copy link
Contributor

@zowoq zowoq commented Jan 11, 2020

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

jonringer
jonringer approved these changes Jan 11, 2020
View reviewed changes
Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

diff LGTM
commit LGTM
executable shows usage

https://github.com/NixOS/nixpkgs/pull/77483
1 package built:
crun

@jonringer
Copy link
Contributor

@GrahamcOfBorg build crun

peterhoeg
peterhoeg requested changes Jan 12, 2020
View reviewed changes
Copy link
Member

@peterhoeg peterhoeg left a comment

Choose a reason for hiding this comment

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

NACK on the source change.

If we start building from the release tarball and therefore remove autoreconfHook it means that people cannot otherwise override the derivation in order to build from their own fork.

Not showing the version is of course sub-optimal but cosmetic - and entirely fixable so address that instead.

@zowoq zowoq closed this Jan 12, 2020
@zowoq zowoq deleted the crun branch January 12, 2020 03:23
@jonringer
Copy link
Contributor

the version could have been patched in, if you still feel like trying to update the package, please re-open :)

example

  sed -i '/#define VERSION/c\#define VERSION ${version}' config.h

@peterhoeg
Copy link
Member

@zowoq, it would be fantastic if you could so the patching as proposed by @jonringer and update the package. We definitely want the latest and greatest! ;-)

@zowoq zowoq restored the crun branch January 13, 2020 09:10
@zowoq zowoq reopened this Jan 13, 2020
@zowoq
Copy link
Contributor Author

zowoq commented Jan 13, 2020

Not using a shallow clone and adding git as a build dep lets the build set tag/version and commit itself.

@zowoq
Copy link
Contributor Author

zowoq commented Jan 15, 2020

Starting with this version the .git dir is required when building from a checkout even if we don't care about setting the version and commit.

Patching the configure and makefile looks non-trivial and might be bit fragile for a pre-1.0.0 upstream.

@zowoq
Copy link
Contributor Author

zowoq commented Jan 16, 2020

The build complains that .git is missing but if you create the file it would have generated if .git existed the build succeeds.

@zowoq zowoq requested a review from peterhoeg January 22, 2020 23:26
@peterhoeg peterhoeg merged commit 2cfb5f5 into NixOS:master Jan 23, 2020
@peterhoeg
Copy link
Member

Thanks @zowoq

@zowoq zowoq deleted the crun branch January 23, 2020 01:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonringer jonringer jonringer approved these changes

@peterhoeg peterhoeg Awaiting requested review from peterhoeg

Assignees
No one assigned
Labels
10.rebuild-darwin: 0 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@zowoq @jonringer @peterhoeg