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

diffoscope: 77 -> 85 #28785

Closed
wants to merge 2 commits into from
Closed

diffoscope: 77 -> 85 #28785

wants to merge 2 commits into from

Conversation

peterhoeg
Copy link
Member

@peterhoeg peterhoeg commented Aug 31, 2017

Motivation for this change

diffoscope is failing on release-17.09 due to a failing test for libarchive-c. The new version of libarchive-c fails the same test so I disabled it, but diffoscope seems to work.

Also includes an update for libarchive-c but diffoscope is the only user of that library.

If @dezgeg is cool with this, I will push it to 17.09 as well.

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

@jerith666
Copy link
Contributor

FYI I bisected the breakage of the test in libarchive-c; it's caused by f9f9749, so cc @fpletz.

@@ -1,25 +1,24 @@
{ lib, stdenv, fetchgit, fetchpatch, python3, docutils
{ lib, stdenv, fetchgit, fetchpatch, python3Packages, docutils
, acl, binutils, bzip2, cbfstool, cdrkit, colord, cpio, diffutils, e2fsprogs, file, fpc, gettext, ghc
, gnupg1, gzip, jdk, libcaca, mono, pdftk, poppler_utils, sng, sqlite, squashfsTools, unzip, vim, xz
Copy link
Member

Choose a reason for hiding this comment

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

I think you meant to replace vim -> xxd (otherwise the reference to xxd below doesn't work).

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, fixed!

@dtzWill
Copy link
Member

dtzWill commented Sep 5, 2017

I was running diffoscope for a long time and was sad there was no progress reporting...
but apparently there is!

After some wrangling the python dependencies, ended up with this: https://github.com/dtzWill/nixpkgs/tree/feature/diffoscope-progressbar

Which also bumps to 86 which among other things adds support for progressbar2.

If this sounds good, feel free to grab the relevant commits or ping me and I'll do a PR in a day or two.

@peterhoeg
Copy link
Member Author

If you could do a PR that would be great! Please ping me when you do the PR.

@peterhoeg peterhoeg mentioned this pull request Sep 6, 2017
@FRidh
Copy link
Member

FRidh commented Sep 6, 2017

I've added these commits to #29009.

@FRidh FRidh closed this Sep 6, 2017
@peterhoeg peterhoeg deleted the fix/diffo branch September 6, 2017 15:01
@dtzWill
Copy link
Member

dtzWill commented Sep 6, 2017

Thanks!

@peterhoeg peterhoeg restored the fix/diffo branch September 7, 2017 00:57
@peterhoeg peterhoeg deleted the fix/diffo branch September 7, 2017 01:43
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