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

i3lock-color: fix manpage-name and manpage #40095

Merged
merged 3 commits into from May 7, 2018
Merged

i3lock-color: fix manpage-name and manpage #40095

merged 3 commits into from May 7, 2018

Conversation

rkoe
Copy link
Contributor

@rkoe rkoe commented May 7, 2018

Motivation for this change

The manpage of i3lock-color should also be named i3lock-color (and not i3lock), and talk about i3lock-color (and not i3lock), so:

  • rename manpage from i3lock.1 to i3lock-color.1
  • change "i3lock" to "i3lock-color" in manpage-synopsis
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.

cc @garbas @malyn

- rename manpage from i3lock.1 to i3lock-color.1
- change "i3lock" to "i3lock-color" in manpage-synopsis
@@ -25,6 +25,8 @@ stdenv.mkDerivation rec {
installFlags = "PREFIX=\${out} SYSCONFDIR=\${out}/etc MANDIR=\${out}/share/man";
postInstall = ''
mv $out/bin/i3lock $out/bin/i3lock-color
mv $out/share/man/man1/i3lock.1 $out/share/man/man1/i3lock-color.1
sed -i 's/^\.B i3lock$/.B i3lock-color/' $out/share/man/man1/i3lock-color.1
Copy link
Contributor

Choose a reason for hiding this comment

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

This sed expression does not catch all instances of i3lock in the manpage, so it results in a mix. Please make sure the entire manpage consistently uses i3lock-color after patching.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I've extended it.

@@ -25,6 +25,8 @@ stdenv.mkDerivation rec {
installFlags = "PREFIX=\${out} SYSCONFDIR=\${out}/etc MANDIR=\${out}/share/man";
postInstall = ''
mv $out/bin/i3lock $out/bin/i3lock-color
mv $out/share/man/man1/i3lock.1 $out/share/man/man1/i3lock-color.1
sed -i 's/^\.B i3lock$/.B i3lock-color/' $out/share/man/man1/i3lock-color.1
'';
meta = with stdenv.lib; {
description = "A simple screen locker like slock";
Copy link
Contributor

Choose a reason for hiding this comment

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

While you're at it, can you please add a hint here on how this differs from regular i3lock, for example
"A simple screen locker like slock, enhanced version with extra color options"
(feel free to come up with a better wording).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok. This will be in a separate pull-request.

complete name-change from i3lock to i3lock-color:

- rename /etc/pam.d/i3lock to /etc/pam.d/i3lock-color
- change "i3lock" to "i3lock-color" everywhere in the manpage
@rkoe
Copy link
Contributor Author

rkoe commented May 7, 2018

Now, I've completely renamed everything:

  • /etc/pam.d/i3lock to i3lock-color
  • every occurence of "i3lock" to "i3lock-color" in the manpage.

Copy link
Contributor

@xeji xeji left a comment

Choose a reason for hiding this comment

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

Manpage looks good now but /etc/pam.d/i3lock should not be renamed.

@@ -25,6 +25,9 @@ stdenv.mkDerivation rec {
installFlags = "PREFIX=\${out} SYSCONFDIR=\${out}/etc MANDIR=\${out}/share/man";
postInstall = ''
mv $out/bin/i3lock $out/bin/i3lock-color
mv $out/etc/pam.d/i3lock $out/etc/pam.d/i3lock-color
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this line as it will cause problems with PAM authentication. The app starts PAM under the service name 'i3lock', so the file must be named /etc/pam.d/i3lock.

Copy link
Contributor

@xeji xeji left a comment

Choose a reason for hiding this comment

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

Thank you, LGTM now.

@xeji
Copy link
Contributor

xeji commented May 7, 2018

@GrahamcOfBorg build i3lock-color

@GrahamcOfBorg
Copy link

No attempt on aarch64-linux (full log)

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

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: i3lock-color

Partial log (click to expand)

glibPreFixupPhase
post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/bcwc9z9c9pva3h5y7ldxlm2icid82xy2-i3lock-color-2.11-c
shrinking /nix/store/bcwc9z9c9pva3h5y7ldxlm2icid82xy2-i3lock-color-2.11-c/bin/i3lock-color
gzipping man pages under /nix/store/bcwc9z9c9pva3h5y7ldxlm2icid82xy2-i3lock-color-2.11-c/share/man/
strip is /nix/store/j75dgadrff2d1fyc4fczmcgqkid2imdx-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/bcwc9z9c9pva3h5y7ldxlm2icid82xy2-i3lock-color-2.11-c/bin
patching script interpreter paths in /nix/store/bcwc9z9c9pva3h5y7ldxlm2icid82xy2-i3lock-color-2.11-c
checking for references to /build in /nix/store/bcwc9z9c9pva3h5y7ldxlm2icid82xy2-i3lock-color-2.11-c...
/nix/store/bcwc9z9c9pva3h5y7ldxlm2icid82xy2-i3lock-color-2.11-c

@xeji xeji merged commit 810976e into NixOS:master May 7, 2018
@GrahamcOfBorg
Copy link

Failure on x86_64-darwin (full log)

Attempted: i3lock-color

Partial log (click to expand)

    ^~~~
    int
1 warning and 3 errors generated.
make[2]: *** [Makefile:683: i3lock-jpg.o] Error 1
make[2]: Leaving directory '/private/tmp/nix-build-i3lock-color-2.11-c.drv-0/source/x86_64-apple-darwin16.3.0'
make[1]: *** [Makefile:444: all] Error 2
make[1]: Leaving directory '/private/tmp/nix-build-i3lock-color-2.11-c.drv-0/source/x86_64-apple-darwin16.3.0'
make: *** [Makefile:426: all] Error 2
builder for '/nix/store/2s3srxyxjw0wxjfpnxjzjyhmj6ybjqgh-i3lock-color-2.11-c.drv' failed with exit code 2
error: build of '/nix/store/2s3srxyxjw0wxjfpnxjzjyhmj6ybjqgh-i3lock-color-2.11-c.drv' failed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants