solc: 0.4.11 -> 0.4.12 #27150
solc: 0.4.11 -> 0.4.12 #27150
Conversation
| stdenv.mkDerivation rec { | ||
| version = "0.4.11"; | ||
| stdenv.mkDerivation { | ||
| inherit version; |
There was a problem hiding this comment.
I believe this shouldn't be needed, I don't think mkDerivation cares about version attribute.
There was a problem hiding this comment.
Ah, thanks. Removing.
| let jsoncpp = fetchzip { | ||
| url = https://github.com/open-source-parsers/jsoncpp/archive/1.7.7.tar.gz; | ||
| url = jsoncppURL; | ||
| sha256 = "0jz93zv17ir7lbxb3dv8ph2n916rajs8i96immwx9vb45pqid3n0"; | ||
| }; in | ||
|
|
There was a problem hiding this comment.
Just a small stylistic comment - was there a reason to prefer this style over:
let rev = "194ff033ae44944ac59aa7bd3da89ba94ec5893c";
sha256 = "0gkg3nay0625qmhxxxax1d1c4dl554ri3pkwd12qfg6g1w6j04w7";
version = "0.4.12";
jsoncppURL = https://github.com/open-source-parsers/jsoncpp/archive/1.7.7.tar.gz;
... rest of the bindings ...
in
which I find bit cleaner compared to nested let expressions.
There was a problem hiding this comment.
Obviously, your version is better. The reason is likely inexperience.
| }; | ||
|
|
||
| patchPhase = '' | ||
| echo >commit_hash.txt ${commit} | ||
| echo >commit_hash.txt ${rev} |
There was a problem hiding this comment.
Requires quotes and almost nobody writes the I/O redirection that way.
There was a problem hiding this comment.
Since they are unary operators, I always write redirections without spaces. And I find it usually makes more sense to put them towards the front of the command.
I'll put in quotes though.
| echo >prerelease.txt | ||
| substituteInPlace deps/jsoncpp.cmake \ | ||
| --replace https://github.com/open-source-parsers/jsoncpp/archive/1.7.7.tar.gz ${jsoncpp} | ||
| --replace ${jsoncppURL} ${jsoncpp} |
There was a problem hiding this comment.
By the way, is there a "shellquote" function? Putting a variable reference in quotes when you know for sure that the value doesn't contain any special characters seems a bit paranoid, especially since theoretically it could still break if there was a quotation mark in the value?
There was a problem hiding this comment.
There was a problem hiding this comment.
Thanks. So you think we should use it here?
There was a problem hiding this comment.
@dbrock No, because you would require a cmake escape function.
There was a problem hiding this comment.
What's a cmake escape function?
There was a problem hiding this comment.
Why should we use the quotes at all to begin with?
Please advise.
There was a problem hiding this comment.
@dbrock There are two levels of code injection possible. One is in the builder (it should be safe to modify the URL to any value) and the other is in CMake.
I am not sure whether this level of correctness is actually achieved by the rest of NixPkgs and assuming you are just interested in getting this into NixPkgs, I am not the one holding back this PR, in case you were thinking that.
|
Should be better now! |
pSub
added
the
8.has: package (update)
Some versions of solc were merged with failing CI builds: NixOS/nixpkgs#20098 NixOS/nixpkgs#27150
Some versions of solc were merged with failing CI builds: NixOS/nixpkgs#20098 NixOS/nixpkgs#27150
Some versions of solc were merged with failing CI builds: NixOS/nixpkgs#20098 NixOS/nixpkgs#27150
Motivation for this change
Things done
(nix.useSandbox on NixOS,
or option
build-use-sandboxinnix.confon non-NixOS)
nix-shell -p nox --run "nox-review wip"./result/bin/)