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

jdupes: init at 1.8 #26920

Merged
merged 1 commit into from Nov 14, 2017
Merged

jdupes: init at 1.8 #26920

merged 1 commit into from Nov 14, 2017

Conversation

romildo
Copy link
Contributor

@romildo romildo commented Jun 28, 2017

Motivation for this change
  • Rename from fdupes to jdupes. The repository fdupes-jody previously
    used does not exist anymore. The fork has been renamed to jdupes.

  • Update to version 1.8.

  • Add jdupes to the package collection.

Changes Release notes

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.

@mention-bot
Copy link

@romildo, thanks for your PR! By analyzing the history of the files in this pull request, we identified @oxij, @MarcWeber and @maggesi to be potential reviewers.

@bennofs
Copy link
Contributor

bennofs commented Jul 17, 2017

@romildo this failed on darwin due to unicode normalization. I pushed a commit that should fix the hash problem, let's see what travis says...

@FRidh
Copy link
Member

FRidh commented Jul 27, 2017

On Darwin

In file included from act_dedupefiles.c:4:

./jdupes.h:23:10: fatal error: 'linux/btrfs.h' file not found

#include <linux/btrfs.h>

         ^~~~~~~~~~~~~~~

1 error generated.

@wmertens
Copy link
Contributor

Should there not be an alias with a deprecation warning?

@FRidh
Copy link
Member

FRidh commented Jul 27, 2017

How significant is this package? Using an alias can offer a smoother transition but how many people use this package and is it worth going through that extra effort (how little it is)

@c0bw3b
Copy link
Contributor

c0bw3b commented Nov 11, 2017

@romildo can you make this PR only about jdupes: init at 1.8 ?

fdupes should not be deprecated yet. Its former home on Google Code has moved to a GH repo and there has been new dev since.
People might be using it in shell scripts and such and the two tools are not interchangeable (different options). So let's keep both in nixpkgs. :)

@romildo romildo changed the title fdupes-1.51 -> jdupes-1.8 jdupes: init at 1.8 Nov 12, 2017
@romildo
Copy link
Contributor Author

romildo commented Nov 12, 2017

can you make this PR only about jdupes: init at 1.8 ?

@c0bw3b Done.

@c0bw3b
Copy link
Contributor

c0bw3b commented Nov 12, 2017

Thanks! You missed @bennofs workaround to avoid src hash being different on Darwin (testdir subdirectories contains unicode characters in their names) :

  src = fetchFromGitHub {
    owner = "jbruchon";
    repo  = "jdupes";
    rev   = "v${version}";
    sha256 = "0nswqylzgpgqgcfgl99dkrcs05zxpzj5dcla2ib9mywl4xavszax";
    postFetch = "$unpackCmd $downloadedFile && rm -r ${name}/testdir";
  };

Also BTRFS support should be enabled on Linux platforms only :

  makeFlags = [ "PREFIX=$(out)" ] ++ stdenv.lib.optional stdenv.isLinux "ENABLE_BTRFS=1";

@c0bw3b c0bw3b mentioned this pull request Nov 12, 2017
8 tasks
@romildo
Copy link
Contributor Author

romildo commented Nov 12, 2017

You missed @bennofs workaround to avoid src hash being different on Darwin.

Also BTRFS support should be enable on Linux platforms only.

Sorry. Fixed now.

@romildo
Copy link
Contributor Author

romildo commented Nov 13, 2017

@bennofs The attribute

postFetch = "$unpackCmd $downloadedFile && rm -r ${name}/testdir";

in src is not working. It is giving the following error:

 /nix/store/bl48w7h24n6379fa200yv60n4pwynd9c-stdenv/setup: line 69: /tmp/nix-build-jdupes-v1.8-src.drv-14/file: Permission denied

It seems that $unpackCmd is expanding to the empty string. What I am missing here?

@romildo
Copy link
Contributor Author

romildo commented Nov 13, 2017

I think removing the testdir directory now works with the following:

  src = fetchFromGitHub {
    owner = "jbruchon";
    repo  = "jdupes";
    rev   = "v${version}";
    sha256 = "17cgh3j6z4rl8ay06s8387a2c49awfv1w3b2a11z4hidwry37aiq";
    # Unicode file names lead to different checksums on HFS+ vs. other
    # filesystems because of unicode normalisation. The testdir
    # directories have such files and will be removed.
    extraPostFetch = "rm -r $out/testdir";
  };

@c0bw3b
Copy link
Contributor

c0bw3b commented Nov 13, 2017

Ah yes you are right it is much nicer this way.
I'm pretty sure the previous patch I struggled to find again was doing just that. :)

Copy link
Contributor

@c0bw3b c0bw3b left a comment

Choose a reason for hiding this comment

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

Now it builds everywhere!

@c0bw3b c0bw3b merged commit 2236363 into NixOS:master Nov 14, 2017
@romildo romildo deleted the upd.fdupes branch November 14, 2017 20:10
@c0bw3b c0bw3b mentioned this pull request Oct 12, 2018
9 tasks
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

9 participants