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

Update hapistrano #72985

Closed
wants to merge 715 commits into from
Closed

Update hapistrano #72985

wants to merge 715 commits into from

Conversation

cptrodolfox
Copy link
Contributor

@cptrodolfox cptrodolfox commented Nov 7, 2019

Motivation for this change

Fix hapistrano broken package.

Things done

Added zsh to list of available build packages for hapistrano in pkgs/development/haskell-modules/configuration-nix.nix.

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • [x ] 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 nix-review --run "nix-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.
Notify maintainers

cc @basvandijk @cdepillabout

chkno and others added 30 commits November 4, 2019 15:11
This is important because this contains some code copied from nix (as an
interim expediency until that functionality can be exported via nix's
API).  The license specified here must be compatible with this borrowing.
Select the same license that nix is released under: lgpl2Plus.
For consistency with the Nix C++ convention.

:~(
I don't think this matters.  As long as one or the other of these is
a std::string, I get an operator== that looks at content rather than
pointer equality.  I picked casting the constant over casting the dynamic
thing in hopes that the compiler would have a better chance at optimizing
away any runtime cost.

Deferring to reviewer.
Switch from convention "appease clang-tidy --checks='*'" to
"References are like non-nullptr pointers".  The clang-tidy check
"google-runtime-references" complains about non-const reference
arguments, but this is not a convention used in Nix.
Switch from convention "appease clang-tidy --checks='*'" to
"References are like non-nullptr pointers".  The clang-tidy check
"google-runtime-references" complains about non-const reference
arguments, but this is not a convention used in Nix.
Switch from convention "appease clang-tidy --checks='*'" to
"References are like non-nullptr pointers".  The clang-tidy check
"google-runtime-references" complains about non-const reference
arguments, but this is not a convention used in Nix.
Automated consumers can use 'sed 1d' or similar to remove this header.

This probably makes this output *easier* to consume correctly.  Having
this header show up in consumers' terminal or log output is probably not
useful, but hiding it without hiding all error messages would have been
more troublesome that just stripping it from stdout.

I.e., previously, unsophisticated use would show undesired output:
  $ some-other-tool
  This attribute set contains:
  This attribute set contains:
  This attribute set contains:
  This attribute set contains:
  <Actual some-other-tool output>

The simplest way to hide this undesired output would have been
nixos-option ... 2>/dev/null, which would hide all error messages.
We do not wish to encourage that.

Correct use would have been something like:
  nixos-option ... 2> >( grep --line-buffered -v 'This attribute set contains:')

After this change, correct use is simpler:
  nixos-option ... | sed 1d
or
  nixos-option ... | sed '1/This attribute set contains:/d'
if the caller don't know if this invocation of nixos-option will yield
an attribute listing or an option description.
Cairo is also a dependency now.

    /tmp/nix-build-inkscape-0.92.4.drv-0/inkscape-0.92.4/src/display/drawing-context.h:20:10: fatal error: 'cairo.h' file not found
    #include <cairo.h>
             ^~~~~~~~~
    1 error generated.
@jonringer
Copy link
Contributor

make sure the branch you branched from matches the base branch :)

@cptrodolfox
Copy link
Contributor Author

cptrodolfox commented Nov 7, 2019

Yeah, just missed that created a new PR #72988.

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