ada-url: init at 2.7.7 #297294
ada-url: init at 2.7.7 #297294
Conversation
NixOSInfra
added
the
12. first-time contribution
ofborg
bot
added
8.has: package (new)
|
Result of 1 package built:
|
You should add yourself directly within this PR, to make it easier to merge! Nixpkgs manual says (on maintainer section):
|
|
Added myself as a maintainer and |
ofborg
bot
added
the
11.by: package-maintainer
|
Result of 1 package built:
|
Based on NixOS#227030 The patch is included since I ran into a roadblock due to how ada implemented cpm (cmake package manager). There are other packages in nixpkgs which work around it (e.g., poac) but those package upstreams gate their downloads on a conditional: ``` if(NOT (EXISTS ${CPM_DOWNLOAD_LOCATION})) file(DOWNLOAD ... ``` I patched ada to reproduce this behavior so it can build in the sandbox.
| - https://github.com/cpm-cmake/CPM.cmake/releases/download/v${CPM_DOWNLOAD_VERSION}/CPM.cmake | ||
| - ${CPM_DOWNLOAD_LOCATION} EXPECTED_HASH SHA256=${CPM_HASH_SUM} | ||
| -) | ||
| +if(NOT (EXISTS ${CPM_DOWNLOAD_LOCATION})) |
There was a problem hiding this comment.
CPM has regular releases and CPM.cmake has a recent update. I don't know if the latest release would solve the problem that this patch solves. If not, the proper action would be to submit a patch to CPM.
Modifying CPM.cmake ourselves is not a good idea. We should rely on releases.
There was a problem hiding this comment.
This is not the "main" CPM.cmake file, but a variation on their provided get_cpm.cmake script. However, I have still filed cpm-cmake/CPM.cmake#554 and if they approve such a change I can also send a patch to ada-url to update their cmake file.
wegank
added
the
12.approvals: 1
|
Can we land this PR? |
| inherit pname version; | ||
| src = fetchFromGitHub { | ||
| owner = "ada-url"; | ||
| repo = pname; |
There was a problem hiding this comment.
This should be explicitly set rather than using pname
| rev = "v1.6.0"; | ||
| hash = "sha256-EAJk3JhLdkuGKRMtspTLejck8doWPd7Z0Lv/Mvf3KFY="; | ||
| }; | ||
| pname = "ada"; |
There was a problem hiding this comment.
Any reason we're using ada here when the attrname is ada-url?
| nativeBuildInputs = [ cmake ]; | ||
|
|
||
| buildInputs = [ cxxopts ]; | ||
| meta = with lib; { |
There was a problem hiding this comment.
| meta = with lib; { | |
| meta = { |
tracking issue: #292468
| pname = "ada"; | ||
| version = "2.7.7"; | ||
| in | ||
| stdenv.mkDerivation rec { |
There was a problem hiding this comment.
| stdenv.mkDerivation rec { | |
| stdenv.mkDerivation { |
|
@ofborg eval |
|
The latest telegram-desktop seems wont build without this package |
@eclairevoyant since the author had no activity in GitHub for the last 4 months, can you commit your suggestions and merge? |
|
Hi @PedroHLC I'm the author of Ada (not this PR), I'm more than happy to help if there's anything I can do. |
|
Yes, go ahead. At the time I wasn't sure what to do based on the upstream response (see above comments), and I've since stopped using Nix. |
Description of changes
Note: I saw the notes on making a separate commit to add oneself to maintainers. I can do that if necessary, but I wasn't sure if maintainers can be empty? I can update this commit to either add myself as a package maintainer or remove the commented line. Besides that line, this PR is ready for review.
Add Ada, "a fast and WHATWG spec compliant URL parser written in C++.
Based on #227030
The patch is included since I ran into a roadblock due to how ada implemented cpm (cmake package manager). There are other packages in nixpkgs which work around it (e.g., poac) but those package upstreams gate their downloads on a conditional:
I patched ada to reproduce this behavior so it can build in the sandbox.
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.