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

s-tar: init at 1.5.3 #40771

Merged
merged 3 commits into from May 28, 2018
Merged

s-tar: init at 1.5.3 #40771

merged 3 commits into from May 28, 2018

Conversation

wucke13
Copy link
Contributor

@wucke13 wucke13 commented May 19, 2018 •
edited

Also added myself to the maintainers list.

Motivation for this change

Needed the package

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-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.

@disassembler
Copy link
Member

@GrahamcOfBorg build standard-tar

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: standard-tar

Partial log (click to expand)

shrinking /nix/store/q9smcccapcnh3m0rzkzrynz8hx58z0a8-star-1.5.3/bin/star
shrinking /nix/store/q9smcccapcnh3m0rzkzrynz8hx58z0a8-star-1.5.3/bin/star_sym
shrinking /nix/store/q9smcccapcnh3m0rzkzrynz8hx58z0a8-star-1.5.3/bin/tartest
gzipping man pages under /nix/store/q9smcccapcnh3m0rzkzrynz8hx58z0a8-star-1.5.3/share/man/
strip is /nix/store/j75dgadrff2d1fyc4fczmcgqkid2imdx-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/q9smcccapcnh3m0rzkzrynz8hx58z0a8-star-1.5.3/lib  /nix/store/q9smcccapcnh3m0rzkzrynz8hx58z0a8-star-1.5.3/bin
patching script interpreter paths in /nix/store/q9smcccapcnh3m0rzkzrynz8hx58z0a8-star-1.5.3
/nix/store/q9smcccapcnh3m0rzkzrynz8hx58z0a8-star-1.5.3/share/doc/star/testscripts/mk: interpreter directive changed from "/bin/sh" to "/nix/store/xn5gv3lpfy91yvfy9b0i7klfcxh9xskz-bash-4.4-p19/bin/sh"
/nix/store/q9smcccapcnh3m0rzkzrynz8hx58z0a8-star-1.5.3/share/doc/star/testscripts/mk2: interpreter directive changed from "/bin/sh" to "/nix/store/xn5gv3lpfy91yvfy9b0i7klfcxh9xskz-bash-4.4-p19/bin/sh"
checking for references to /build in /nix/store/q9smcccapcnh3m0rzkzrynz8hx58z0a8-star-1.5.3...

@GrahamcOfBorg
Copy link

Failure on x86_64-darwin (full log)

Attempted: standard-tar

Partial log (click to expand)

checking whether /bin/bosh is a working shell... no
checking whether /opt/schily/bin/bosh is a working shell... no
checking for cc... clang
checking for gcc... (cached) clang
checking whether the C compiler (clang   -O   -Llibs/i386-darwin-clang -L/usr/local/lib -framework IOKit -framework CoreFoundation) works... no
configure: error: installation or configuration problem: C compiler cannot create executables.
RULES/rules.cnf:70: incs/i386-darwin-clang/rules.cnf: No such file or directory
make: *** [RULES/rules.cnf:59: incs/i386-darwin-clang/rules.cnf] Error 1
builder for '/nix/store/zrj0vhrs84831nw7r68pkkc7ai898cq2-star-1.5.3.drv' failed with exit code 2
error: build of '/nix/store/zrj0vhrs84831nw7r68pkkc7ai898cq2-star-1.5.3.drv' failed

@GrahamcOfBorg
Copy link

Failure on aarch64-linux (full log)

Attempted: standard-tar

Partial log (click to expand)

        ==> GENERATING include file "../incs/aarch64-linux-cc/align.h"
/nix/store/l4w7xwjy2nmk31fl5kgyy7gg8z7l9n8z-bash-4.4-p19/bin/bash: line 1: OBJ/aarch64-linux-cc/align_test: No such file or directory
/nix/store/l4w7xwjy2nmk31fl5kgyy7gg8z7l9n8z-bash-4.4-p19/bin/bash: line 2: OBJ/aarch64-linux-cc/align_test: No such file or directory
make[2]: *** [../RULES/rules.inc:50: ../incs/aarch64-linux-cc/align.h] Error 127
make[2]: Leaving directory '/build/star-1.5.3/inc'
make[1]: *** [../RULES/rules.mks:23: install] Error 2
make[1]: Leaving directory '/build/star-1.5.3/inc'
make: *** [RULES/rules1.dir:25: install] Error 2
builder for '/nix/store/rp9b32a796ikyb2cghijyzn99c56k4zf-star-1.5.3.drv' failed with exit code 2
�[31;1merror:�[0m build of '/nix/store/rp9b32a796ikyb2cghijyzn99c56k4zf-star-1.5.3.drv' failed

rycee
rycee reviewed May 20, 2018
View reviewed changes
pkgs/tools/archivers/star/default.nix Outdated
In all cases, appearance of a directory name refers to the files and (recursively) sub-directories of that directory.
Star's actions are controlled by the mandatory command flags from the list below.
The way star acts may be modified by additional options.
Note that unpacking tar archives may be a security risk because star may overwrite existing files.
Copy link
Member

Choose a reason for hiding this comment

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

These lines should all be indented:

    longDescription = ''
      Star archives and extracts multiple files to and from a single file called a tarfile.
      …
      Note that unpacking tar archives may be a security risk because star may overwrite existing files.
    '';

rycee
rycee reviewed May 20, 2018
View reviewed changes
pkgs/tools/archivers/star/default.nix Outdated
name = "star-${version}";
version = "1.5.3";
src = fetchurl {
url = "https://sourceforge.net/projects/s-tar/files/${name}.tar.gz";
Copy link
Member

Choose a reason for hiding this comment

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

Should use mirror://sourceforge. Something like

url = "mirror://sourceforge/s-tar/${name}.tar.gz";

might work.

There also seems to be a tar.bz2 download file as well so can use that. Saves a few bytes :-)

rycee
rycee reviewed May 20, 2018
View reviewed changes
pkgs/tools/archivers/star/default.nix Outdated

preConfigure = "rm configure";
preBuild = "sed 's_/bin/__g' -i RULES/*";
makeFlags = [ "DESTDIR=$(out)" "INS_BASE=/ install"];
Copy link
Member

Choose a reason for hiding this comment

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

  makeFlags = [ "GMAKE_NOWARN=true" ];
  installFlags = [ "DESTDIR=$(out)" "INS_BASE=/" ];

rycee
rycee reviewed May 20, 2018
View reviewed changes
pkgs/tools/archivers/star/default.nix Outdated
makeFlags = [ "DESTDIR=$(out)" "INS_BASE=/ install"];
postInstall = ''
find $out/bin -type l -delete
rm -r $out/etc $out/include $out/sbin
Copy link
Member

Choose a reason for hiding this comment

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

Need to indent a bit:

  postInstall = ''
    find $out/bin -type l -delete
    rm -r $out/etc $out/include $out/sbin
  '';

rycee
rycee reviewed May 20, 2018
View reviewed changes
pkgs/tools/archivers/star/default.nix Outdated
'';

meta = {
description = "Star is a very fast tar like tape archiver with improved functionality.";
Copy link
Member

Choose a reason for hiding this comment

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

No need to start the description with the package name and it should not end with a period. Also there is a double space here.

Perhaps change to

description = "A very fast tar like tape archiver with improved functionality";

@rycee
Copy link
Member

rycee commented May 20, 2018

Based on the GrahamcOfBorg build result it might be worth limiting this package to x86 linux platforms.

@wucke13
Copy link
Contributor Author

wucke13 commented May 20, 2018

@rycee Thanks for your detailed feedback, I really appreciate it! I made all the changes you mentioned but the platforms thing. How do I specify that it's only platform is x86 linux?

@wucke13
standard-tar: init at 1.5.3
deae772 
Also added myself to the maintainers list.
@xeji
Copy link
Contributor

xeji commented May 22, 2018

How do I specify that it's only platform is x86 linux?

platforms = [ "x86_64-linux" ];

@wucke13
Copy link
Contributor Author

wucke13 commented May 23, 2018

@rycee @xeji Thanks for the feedback. Is there something still missing?

@xeji
Copy link
Contributor

xeji commented May 23, 2018

Content looks good to me, but the naming is not yet consistent: the attribute name is standard-tar, package name star, nixpkgs folder star, upstream calls it Star or S tar, sourceforce directory is s-tar. And there's another package called star (attribute and package) in nixpkgs.

We should try to be as consistent as possible to avoid confusion. Ideally, attribute, folder, and package have the same name (which cannot be star as that's already taken). The name should be as close as possible to the official name given by upstream.

My personal preference would be s-tar. But that's just a matter of taste - entirely your choice.

@rycee
Copy link
Member

rycee commented May 23, 2018

Looks good to me but I agree about the naming. My opinion is also that s-tar would be nice to have everywhere.

To whoever merges: remember to squash.

@wucke13
Copy link
Contributor Author

wucke13 commented May 24, 2018

Hmmh, that's an interesting problem. Both the executable and the software itself is named star (or Star). The reason it's called s-tar in the path on sourceforge is unclear to me, however the meta attribute of name also is star. If slightly changing the second ones name is the way to go on a duplicate name, then s-tar look like a decent choice to me too.

@xeji
Copy link
Contributor

xeji commented May 24, 2018

Fine. If you rename everything consistently to s-tar this should be ready to merge.

@xeji
Copy link
Contributor

xeji commented May 28, 2018

@GrahamcOfBorg build s-tar

@GrahamcOfBorg
Copy link

No attempt on aarch64-linux (full log)

The following builds were skipped because they don't evaluate on aarch64-linux: s-tar

Partial log (click to expand)


a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowUnsupportedSystem = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowUnsupportedSystem = true; }
to ~/.config/nixpkgs/config.nix.

@GrahamcOfBorg
Copy link

No attempt on x86_64-darwin (full log)

The following builds were skipped because they don't evaluate on x86_64-darwin: s-tar

Partial log (click to expand)


a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowUnsupportedSystem = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowUnsupportedSystem = true; }
to ~/.config/nixpkgs/config.nix.

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: s-tar

Partial log (click to expand)

shrinking /nix/store/7y6qhmii1l6n0xnzi9ksj7dfi8xzpnzn-s-tar-1.5.3/bin/star
shrinking /nix/store/7y6qhmii1l6n0xnzi9ksj7dfi8xzpnzn-s-tar-1.5.3/bin/star_sym
shrinking /nix/store/7y6qhmii1l6n0xnzi9ksj7dfi8xzpnzn-s-tar-1.5.3/bin/tartest
gzipping man pages under /nix/store/7y6qhmii1l6n0xnzi9ksj7dfi8xzpnzn-s-tar-1.5.3/share/man/
strip is /nix/store/92d2ifxcni4n3zx9s8wnkcjlvnx5ajlc-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/7y6qhmii1l6n0xnzi9ksj7dfi8xzpnzn-s-tar-1.5.3/lib  /nix/store/7y6qhmii1l6n0xnzi9ksj7dfi8xzpnzn-s-tar-1.5.3/bin
patching script interpreter paths in /nix/store/7y6qhmii1l6n0xnzi9ksj7dfi8xzpnzn-s-tar-1.5.3
/nix/store/7y6qhmii1l6n0xnzi9ksj7dfi8xzpnzn-s-tar-1.5.3/share/doc/star/testscripts/mk: interpreter directive changed from "/bin/sh" to "/nix/store/m47apl3hq3i52fzy2cz24378p0xn4lyx-bash-4.4-p19/bin/sh"
/nix/store/7y6qhmii1l6n0xnzi9ksj7dfi8xzpnzn-s-tar-1.5.3/share/doc/star/testscripts/mk2: interpreter directive changed from "/bin/sh" to "/nix/store/m47apl3hq3i52fzy2cz24378p0xn4lyx-bash-4.4-p19/bin/sh"
checking for references to /build in /nix/store/7y6qhmii1l6n0xnzi9ksj7dfi8xzpnzn-s-tar-1.5.3...

@xeji
Copy link
Contributor

xeji commented May 28, 2018

Thank you!

@xeji xeji changed the title standard-tar: init at 1.5.3 s-tar: init at 1.5.3 May 28, 2018
@xeji xeji merged commit 347cc50 into NixOS:master May 28, 2018
@peti
Copy link
Member

peti commented Jun 5, 2018

This package causes evaluation errors:

$ nix-env -qaP --xml --meta 2>&1 >/dev/null
derivation 's-tar-1.5.3' has invalid meta attribute 'override'
derivation 's-tar-1.5.3' has invalid meta attribute 'overrideDerivation'

@peti peti mentioned this pull request Jun 5, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rycee rycee rycee left review comments

Assignees
No one assigned
Labels
8.has: package (new) 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
6 participants
@wucke13 @disassembler @GrahamcOfBorg @rycee @xeji @peti