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

Add git sync #41876

Merged
merged 3 commits into from Jun 26, 2018
Merged

Add git sync #41876

merged 3 commits into from Jun 26, 2018

Conversation

colonelpanic8
Copy link
Contributor

@colonelpanic8 colonelpanic8 commented Jun 12, 2018 •
edited

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

This is my the first derivation I've added, so let me know if anything looks strange.

I'm sure this is explained somewhere, but its not clear to me how useSandbox should be used, or where it should be set. The build section of this package is literally just a copy, but i'd still like to know how to do this.

ryantm
ryantm requested changes Jun 12, 2018
View reviewed changes
pkgs/tools/backup/git-sync/default.nix Outdated
}:

stdenv.mkDerivation rec {
name = "git-sync";
Copy link
Member

Choose a reason for hiding this comment

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

The version should be part of the name like "name-${version}". In the case of a project like this with no releases, we typically use the date the commit was made Oct 24, 2015, so we'd use 20151024 as the version. This would let us have increasing version numbers when we make updates to the package. (Assuming updates are continuing for this package!)

Copy link
Contributor

Choose a reason for hiding this comment

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

The version should probably be unstable-2015-10-24, see https://nixos.org/nixpkgs/manual/#sec-package-naming.

@colonelpanic8
Copy link
Contributor Author

@ryantm Is there a better way to test the resulting binary than using

nix-build -A git-sync -K

And then testing what is produced in the result directory?

@xeji
Copy link
Contributor

xeji commented Jun 12, 2018

This package would better fit in the pkgs/applications/version-management/git-and-tools directory, which contains all git related tools.

@ryantm
Copy link
Member

ryantm commented Jun 12, 2018

@IvanMalison You could build it in a more restricted way:

nix-build --option sandbox true --option restrict-eval true -A git-sync -K

Can you change the commit message to say "git-sync: init at 20151024"?

I think xeji is right about the file location too.

@ryantm
Copy link
Member

ryantm commented Jun 13, 2018

@GrahamcOfBorg build git-sync

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: git-sync

Partial log (click to expand)

source root is source
patching sources
updateAutotoolsGnuConfigScriptsPhase
configuring
no configure script, doing nothing
installing
post-installation fixup
patching script interpreter paths in /nix/store/zkpw89dxkkwp5dwyvjazv1qx2df2qsjw-git-sync-20151024/bin
/nix/store/zkpw89dxkkwp5dwyvjazv1qx2df2qsjw-git-sync-20151024/bin/git-sync: interpreter directive changed from "/usr/bin/env bash" to "/nix/store/9rv2bzx7jz6wjbaxij5qq580qvzksj4i-bash-4.4-p19/bin/bash"
/nix/store/zkpw89dxkkwp5dwyvjazv1qx2df2qsjw-git-sync-20151024

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: git-sync

Partial log (click to expand)

unpacking source archive /nix/store/mq5m03kfalv5my77934l8gqz1v0hc8zd-source
source root is source
patching sources
configuring
no configure script, doing nothing
installing
post-installation fixup
patching script interpreter paths in /nix/store/ywasxkp7jg4yxnai6kwdks7w3jx6l9yz-git-sync-20151024/bin
/nix/store/ywasxkp7jg4yxnai6kwdks7w3jx6l9yz-git-sync-20151024/bin/git-sync: interpreter directive changed from "/usr/bin/env bash" to "/nix/store/qckzjk3406va7h6s40cy9s75z2w715rq-bash-4.4-p19/bin/bash"
/nix/store/ywasxkp7jg4yxnai6kwdks7w3jx6l9yz-git-sync-20151024

@GrahamcOfBorg
Copy link

Success on x86_64-darwin (full log)

Attempted: git-sync

Partial log (click to expand)

unpacking source archive /nix/store/mq5m03kfalv5my77934l8gqz1v0hc8zd-source
source root is source
patching sources
configuring
no configure script, doing nothing
installing
post-installation fixup
patching script interpreter paths in /nix/store/bzflg4s6myvqhvdxf2qwfh4i2nls4z7h-git-sync-20151024/bin
/nix/store/bzflg4s6myvqhvdxf2qwfh4i2nls4z7h-git-sync-20151024/bin/git-sync: interpreter directive changed from "/usr/bin/env bash" to "/nix/store/9gghkkqhn6v7dvpf2319qvrxkkw03vwr-bash-4.4-p19/bin/bash"
/nix/store/bzflg4s6myvqhvdxf2qwfh4i2nls4z7h-git-sync-20151024

@colonelpanic8
Copy link
Contributor Author

@ryantm I think that in this case, sinceall building involves copying a file, the more important test is at runtime.

I tried running the resulting wrapped binary with PATH="" and things seemed to work.
Is there are more nixosy way to do this?

@ryantm
Copy link
Member

ryantm commented Jun 13, 2018

@IvanMalison You could try nix-shell --pure -p git-sync

@colonelpanic8
Copy link
Contributor Author

@ryantm Things look good.

Is something blocking merge?

@colonelpanic8 colonelpanic8 mentioned this pull request Jun 26, 2018
Closed
9 tasks
@xeji
git-sync: move to gitAndTools.git-sync
19ce965 
By convention, git-related tools in nixpkgs are not added to
the top level but bundled under gitAndTools.
@xeji
Copy link
Contributor

xeji commented Jun 26, 2018

@GrahamcOfBorg build gitAndTools.git-sync

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: gitAndTools.git-sync

Partial log (click to expand)

source root is source
patching sources
updateAutotoolsGnuConfigScriptsPhase
configuring
no configure script, doing nothing
installing
post-installation fixup
patching script interpreter paths in /nix/store/5102bs0fk2m0knzshr3iacbk2kc3fyqn-git-sync-20151024/bin
/nix/store/5102bs0fk2m0knzshr3iacbk2kc3fyqn-git-sync-20151024/bin/git-sync: interpreter directive changed from "/usr/bin/env bash" to "/nix/store/p0vy17dp9jk2mvqsxsqnb14s3797lay7-bash-4.4-p23/bin/bash"
/nix/store/5102bs0fk2m0knzshr3iacbk2kc3fyqn-git-sync-20151024

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: gitAndTools.git-sync

Partial log (click to expand)

unpacking source archive /nix/store/mq5m03kfalv5my77934l8gqz1v0hc8zd-source
source root is source
patching sources
configuring
no configure script, doing nothing
installing
post-installation fixup
patching script interpreter paths in /nix/store/l8n3azg54lv31i8c5nbwn0s18dzwn47p-git-sync-20151024/bin
/nix/store/l8n3azg54lv31i8c5nbwn0s18dzwn47p-git-sync-20151024/bin/git-sync: interpreter directive changed from "/usr/bin/env bash" to "/nix/store/8zkg9ac4s4alzyf4a8kfrig1j73z66dw-bash-4.4-p23/bin/bash"
/nix/store/l8n3azg54lv31i8c5nbwn0s18dzwn47p-git-sync-20151024

@xeji
Copy link
Contributor

xeji commented Jun 26, 2018

@IvanMalison By convention, git-related tools in nixpkgs are not added to
the top level but bundled under gitAndTools. I added a commit to move it there.
It can now be installed as gitAndTools.git-sync.

@xeji xeji merged commit b8ad7ec into NixOS:master Jun 26, 2018
@GrahamcOfBorg
Copy link

Success on x86_64-darwin (full log)

Attempted: gitAndTools.git-sync

Partial log (click to expand)

unpacking source archive /nix/store/mq5m03kfalv5my77934l8gqz1v0hc8zd-source
source root is source
patching sources
configuring
no configure script, doing nothing
installing
post-installation fixup
patching script interpreter paths in /nix/store/ki845cvn6garxi6l7bdwjj3msm0hfzg5-git-sync-20151024/bin
/nix/store/ki845cvn6garxi6l7bdwjj3msm0hfzg5-git-sync-20151024/bin/git-sync: interpreter directive changed from "/usr/bin/env bash" to "/nix/store/q2wqq1k20v8kc3vckapqf5nws30brnni-bash-4.4-p23/bin/bash"
/nix/store/ki845cvn6garxi6l7bdwjj3msm0hfzg5-git-sync-20151024

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

@jtojnar jtojnar jtojnar left review comments

@ryantm ryantm ryantm requested changes

@xeji xeji xeji approved these changes

Assignees
No one assigned
Labels
8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 12. first-time contribution
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@colonelpanic8 @xeji @ryantm @GrahamcOfBorg @jtojnar @Janik-Haag