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

tig: 2.2 -> 2.2.1 #24770

Merged
merged 2 commits into from Apr 10, 2017
Merged

tig: 2.2 -> 2.2.1 #24770

merged 2 commits into from Apr 10, 2017

Conversation

sigma
Copy link
Contributor

@sigma sigma commented Apr 9, 2017

Motivation for this change

Catch up with upstream.

Also move to different project URLs, as requested in
https://github.com/jonas/tig/releases/tag/tig-2.2.1

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
    • Linux
  • 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.

Also move to different project URLs, as requested in
https://github.com/jonas/tig/releases/tag/tig-2.2.1
@mention-bot
Copy link

@sigma, thanks for your PR! By analyzing the history of the files in this pull request, we identified @garbas, @dmalikov and @bjornfor to be potential reviewers.

url = "http://jonas.nitro.dk/tig/releases/${name}.tar.gz";
sha256 = "0k3m894vfkgkj7xbr0j6ph91351dl6id5f0hk2ksjp5lmg9i6llg";
url = "https://github.com/jonas/tig/releases/download/${name}/${name}.tar.gz";
sha256 = "0wgsqdly3jd9c7b0ibb5vwsv4js19c5hi0698nf1fnfyjq40hj0b";
};

buildInputs = [ ncurses asciidoc xmlto docbook_xsl readline git makeWrapper ]
Copy link
Contributor

Choose a reason for hiding this comment

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

makeWrapper belongs in nativeBuildInputs


src = fetchurl {
url = "http://jonas.nitro.dk/tig/releases/${name}.tar.gz";
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you tried fetchFromGitHub, as it is now the preferred method when possible. When you do it, rev is most likely gonna be:
rev = "v${version}"; sometimes it is rev = version; according how the upstream lists their version

@sigma
Copy link
Contributor Author

sigma commented Apr 10, 2017

@ndowens that was a little bit more involved than I thought (turns out the .tar.gz was a pre-configured version of the sources), but I think it should be ok now.

@sigma sigma force-pushed the pr/tig branch 2 times, most recently from 192054e to 229670d Compare April 10, 2017 01:16
@sigma
Copy link
Contributor Author

sigma commented Apr 10, 2017

hmm, not sure what's going on with the OSX build (which works in my own setup)

@sigma sigma force-pushed the pr/tig branch 4 times, most recently from 2c63965 to f6f7504 Compare April 10, 2017 02:14
@sigma
Copy link
Contributor Author

sigma commented Apr 10, 2017

ok, I'm finally able to reproduce, by uninstalling the docbook-xsl homebrew package... something is seriously wrong right there.

@sigma
Copy link
Contributor Author

sigma commented Apr 10, 2017

and what was wrong was that tig's Makefile kinda goes out of its way to make it's as impure as possible :)
Interesting learning experience at least !

@peterhoeg
Copy link
Member

Instead of adding automake/autoconf and manually running ./autogen.sh, you can probably add autoreconfHook to nativeBuildInputs. That should take care of it automatically.

@ndowens
Copy link
Contributor

ndowens commented Apr 10, 2017

I tried autoreconfHook but it didn't work

Rework the dependencies to allow use of fetchFromGitHub.
@sigma
Copy link
Contributor Author

sigma commented Apr 10, 2017

@peterhoeg thanks, that seems to do the trick. @ndowens am I missing something?

@Mic92 Mic92 merged commit a78ce1d into NixOS:master Apr 10, 2017
@Mic92
Copy link
Member

Mic92 commented Apr 10, 2017

Thanks!

@sigma sigma deleted the pr/tig branch April 16, 2017 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants