haskellPackages.hcoord: fix build

[erictapen]

danfran/hcoord#8 increases version bounds for hcoord. For some reason the build fails when the change is applied with appendPatch, so let's use my branch. The Git tag says version v1.0.0.0 is the same rev as master, so there should be no difference between my branch and master+appendPatch, but however.

Motivation for this change

Things done

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

[cdepillabout]

@erictapen Thanks for trying to get this fixed.

Two things:

1. Can you send these Haskell-related updates to the haskell-updates branch, instead of master?
2. Normally we prefer to wait for upstream to just make a new release fixing these types of problems, since it is less work for us. Can you wait a week, and if upstream doesn't respond or create a new release, then we can come back to this PR. Also, in the future, we prefer to call fetchpatch on the commit or PR responsible, rather than just updating the underlying source. There should some examples of using fetchpatch for this in the configuration.nix file you can copy.

[erictapen]

1. Right, forget this everytime.
2. I explained in the commit message and PR text why I'd rather use overrideSrc.

Right, let's wait a while and see how upstream responds.

[peti]

Please don't override the source for the entire package. Instead, please apply the necessary changes as patches to the official release tarball. This approach has the advantage that the patches will fail when a new version comes out and this is a good reminder for us to remove the override.

[peti]

Ping? Will you address the comments?

[erictapen]

I adressed your comments and rebased to latest haskell-updates. Also updated the commit message.

Edit: Also one meta comment: I very much agree with your reasoning about preffering appendPatch vs. using a different branch and as you can see, I switched to using the upstream source. But it would've helped me to see my reasoning stated above being addressed, so it wouldn't feel like you didn't read my PR text and the commit message at all. Something as short as "Besides your reasons I still think it's a bad idea." would have made the review easier to digest for me.

[cdepillabout]

It would probably be easier to just mark this package as dontCheck, rather than overriding the upstream source.

The problem with overriding the upstream source is that we will have to remember to undo it at some point. If we don't undo it, we will continue getting the old version.

[erictapen]

@cdepillabout Mh I get it. But I don't like disabling tests when they could be easily run. How about this:

    src = pkgs.fetchFromGitHub {
      owner = "danfran";
      repo = drv.pname;
      rev = "v${drv.version}";
      sha256 = "16a7q2nz0k0z96yyyc9s5pblxaznb9mqj94k8md2a51f36f1vaw4";
    }

By using drv.version we guarantee to use the corresponding source to the Hackage release. If upstream forgets to create the tag, we get notified. We won't get notified when upstream fixes danfran/hcoord#9, but the tests would still run and we won't get notified either when using dontCheck.

[cdepillabout]

I think this still suffers from the fixed-output derivation problem, and we still don't want to override the source of the upstream tarball.

But it would've helped me to see my reasoning stated above being addressed

From your original PR message, it seemed like your reasoning was the following:

For some reason the build fails when the change is applied with appendPatch

The "for some reason" part doesn't sound like a valid reason not to use appendPatch. If you don't understand why something doesn't work, definitely freel free to ask us for advice how to get it to work. (A detailed description of what you tried and what error messages you get makes it easier to help you as well!)

[erictapen]

@cdepillabout Please note that I wasn't criticizing your rejection of my change, but the way you communicated it first in #93469 (comment).

I force-pushed and addressed your concerns.

[maralorn]

@GrahamcOfBorg build haskellPackages.hcoord

[maralorn]

@GrahamcOfBorg build haskellPackages.hcoord

[maralorn]

Tested locally, works fin.

[maralorn]

Thx, for the PR!