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.

Sign up for GitHub

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

Jump to bottom

photivo: fix build with lensfun >= 0.3 #23499

Closed
wants to merge 1 commit into from
Closed

photivo: fix build with lensfun >= 0.3 #23499

wants to merge 1 commit into from

Conversation

rycee
Copy link
Member

@rycee rycee commented Mar 4, 2017

Motivation for this change

To fix photivo build in master and 17.03.

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.

@mention-bot
Copy link

@rycee, thanks for your PR! By analyzing the history of the files in this pull request, we identified @vcunat, @bennofs and @pSub to be potential reviewers.

the-kenny
the-kenny reviewed Mar 5, 2017
View reviewed changes
pkgs/applications/graphics/photivo/default.nix Outdated
@@ -10,6 +10,14 @@ stdenv.mkDerivation rec {
sha256 = "0f6y18k7db2ci6xn664zcwm1g1k04sdv7gg1yd5jk41bndjb7z8h";
};

patches = [
(fetchpatch {
url = "https://www.linuxquestions.org/questions/attachment.php?attachmentid=17287&d=1420577220";
Copy link
Contributor

Choose a reason for hiding this comment

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

I have a feeling that this is better served by including the file in nixpkgs (with a reference to the original source) rather than downloading it from a comment in a forum post.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I agree. I have force pushed a new version that includes the patch directly. Thanks for the comment!

@the-kenny
Copy link
Contributor

For some reason the current commit is broken for me:

building path(s) ‘/nix/store/x0c2w5wzwn87vgfrzwxl9pz0v7riqhhc-photivo-2014-01-25’
unpacking sources
unpacking source archive /nix/store/my4wwhj7id1aqn0a95vqj7m70601qfl9-hg-archive
source root is hg-archive
patching sources
applying patch /nix/store/aswza9pn5mwxjj3w4jjygxa1ggyh3cvy-lensfun-0.3.patch
patching file Sources/ptConstants.h
Hunk #1 FAILED at 222 (different line endings).
1 out of 1 hunk FAILED -- saving rejects to file Sources/ptConstants.h.rej
patching file Sources/ptGuiOptions.cpp
Hunk #1 FAILED at 177 (different line endings).
1 out of 1 hunk FAILED -- saving rejects to file Sources/ptGuiOptions.cpp.rej
patching file Sources/ptMainWindow.cpp
Hunk #1 FAILED at 3067 (different line endings).
1 out of 1 hunk FAILED -- saving rejects to file Sources/ptMainWindow.cpp.rej
patching file Sources/ptProcessor.cpp
builder for ‘/nix/store/387vdx6yd47abkzr2d0x83w9di4cd0c2-photivo-2014-01-25.drv’ failed with exit code 1
error: build of ‘/nix/store/387vdx6yd47abkzr2d0x83w9di4cd0c2-photivo-2014-01-25.drv’ failedbuilding path(s) ‘/nix/store/x0c2w5wzwn87vgfrzwxl9pz0v7riqhhc-photivo-2014-01-25’
unpacking sources
unpacking source archive /nix/store/my4wwhj7id1aqn0a95vqj7m70601qfl9-hg-archive
source root is hg-archive
patching sources
applying patch /nix/store/aswza9pn5mwxjj3w4jjygxa1ggyh3cvy-lensfun-0.3.patch
patching file Sources/ptConstants.h
Hunk #1 FAILED at 222 (different line endings).
1 out of 1 hunk FAILED -- saving rejects to file Sources/ptConstants.h.rej
patching file Sources/ptGuiOptions.cpp
Hunk #1 FAILED at 177 (different line endings).
1 out of 1 hunk FAILED -- saving rejects to file Sources/ptGuiOptions.cpp.rej
patching file Sources/ptMainWindow.cpp
Hunk #1 FAILED at 3067 (different line endings).
1 out of 1 hunk FAILED -- saving rejects to file Sources/ptMainWindow.cpp.rej
patching file Sources/ptProcessor.cpp
builder for ‘/nix/store/387vdx6yd47abkzr2d0x83w9di4cd0c2-photivo-2014-01-25.drv’ failed with exit code 1
error: build of ‘/nix/store/387vdx6yd47abkzr2d0x83w9di4cd0c2-photivo-2014-01-25.drv’ failed

@the-kenny
Copy link
Contributor

the-kenny commented Mar 5, 2017 •
edited

Note that this might be caused by git modifying the patche's line endings:

% curl 'https://www.linuxquestions.org/questions/attachment.php?attachmentid=17287&d=1420577220' > /tmp/foo
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  2918  100  2918    0     0   4148      0 --:--:-- --:--:-- --:--:--  4144
moritz@moritz-x260 ~/dev/nixpkgs (git)-[master] % file /tmp/foo
/tmp/foo: unified diff output, ASCII text, with CRLF, LF line terminators

% file pkgs/applications/graphics/photivo/lensfun-0.3.patch
pkgs/applications/graphics/photivo/lensfun-0.3.patch: unified diff output, ASCII text

I'm not sure how to disable this behavior in git though.

@rycee
Copy link
Member Author

rycee commented Mar 5, 2017

Well, isn't that interesting… Seems like git will change the line ending unless you have a global configuration set or use a gitattributes file. I'm reluctant to add a gitattributes file just for this file.

So I'm inclined to just use the forum source. It's been a stable link for like two years so hopefully it will stick around for longer. Alternatively, can use https://github.com/driesz/photivo/ as upstream, it seems to include this fix and some other stuff. Don't know how official it is, though.

@the-kenny
Copy link
Contributor

@rycee Agreed. Let's just use the forum source. Git should really have a global blacklist for things like patchfiles.

@rycee
Copy link
Member Author

rycee commented Mar 6, 2017

Yeah, keeping precise newlines on *.patch and *.diff might be a good idea. That's for another time and another PR though :-)

I switched back to downloading the patch from the forum.

@grahamc
Copy link
Member

grahamc commented Mar 6, 2017

Applied in 747b62f, thank you!

@grahamc
Copy link
Member

grahamc commented Mar 6, 2017

Applied to 17.03 in 9279742

@grahamc grahamc closed this Mar 6, 2017
@rycee rycee deleted the fix/photivo branch March 6, 2017 22:46
@rycee
Copy link
Member Author

rycee commented Mar 6, 2017

Cheers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@the-kenny the-kenny the-kenny left review comments

Assignees
No one assigned
Labels
0.kind: bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@rycee @mention-bot @the-kenny @grahamc