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
sqlint: init at 0.1.7 #41901
sqlint: init at 0.1.7 #41901
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please squash this into your other commit with the same commit message
6947470
to
6214d18
Compare
@@ -1489,6 +1489,8 @@ with pkgs; | |||
utillinux = utillinuxMinimal; | |||
}; | |||
|
|||
sqlint = callPackage ../development/tools/sqlint { }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure how these are intended to be sorted. Why do we go back to a
below here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sometimes packages of similar category are kept together. Sometimes they get out of order due merges being delayed.
We still try to keep them alphabetical since this avoids merge conflicts.
Squash complete. |
|
||
gemConfig = { | ||
pg_query = attrs: { | ||
NIX_SSL_CERT_FILE = "${cacert}/etc/ssl/certs/ca-bundle.crt"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The package tries to download source code at runtime: https://github.com/lfittl/pg_query/blob/de2c9d1f61d7be1d2ba8be24bc495221cbc626a1/ext/pg_query/extconf.rb#L13
This will not work on hydra since we disallow network access in our build sandbox. However it is possible to pre-download the source with fetchurl instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you recommend I create a separate package for pg_query, or should I just keep it all in this package?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Usually I would recommend to put it into https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/ruby-modules/gem-config/default.nix
However in this case here it is tricky because it seems to require a specific version of libpg_query.
In this case better to keep the override for pg_query in this file because the version for it is fixed until sqlint receives an upgrade.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Getting the libpg_query
tarball is easy enough:
libpgQuerySrc = fetchurl {
url = https://codeload.github.com/lfittl/libpg_query/tar.gz/10-1.0.1;
sha256 = "0m5jv134hgw2vcfkqlnw80fr3wmrdvgrvk1ndcx9s44bzi5nsp47";
};
But I can't seem to copy it to the path corresponding to "#{workdir}/libpg_query.tar.gz"
before the build starts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried the following in preConfigure
, postConfigure
, preInstall
, etc., without success:
mkdir -p $out/lib/ruby/gems/2.4.0/gems/pg_query-0.13.5/ext/pg_query
cp ${libpgQuerySrc} $out/lib/ruby/gems/2.4.0/gems/pg_query-0.13.5/ext/pg_query/libpg_query.tar.gz
@GrahamcOfBorg build sqlint |
Failure on aarch64-linux (full log) Attempted: sqlint Partial log (click to expand)
|
Failure on x86_64-linux (full log) Attempted: sqlint Partial log (click to expand)
|
Success on x86_64-darwin (full log) Attempted: sqlint Partial log (click to expand)
|
The linux build failures will likely be resolved if we can figure out how to download ahead of time and install the |
#patches = [./dir.patch]; | ||
postConfigure = '' | ||
mkdir -p $out/${attrs.ruby.gemPath}/gems/pg_query-${pgQueryVersion}/ext/pg_query | ||
cp ${libpgQuerySrc} $out/${attrs.ruby.gemPath}/gems/pg_query-${pgQueryVersion}/ext/pg_query/libpg_query.tar.gz |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
libpg_query.tar.gz should be in place here for the install phase, but it's not.
|
||
gemConfig = { | ||
pg_query = attrs: { | ||
#patches = [./dir.patch]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried adding a patch to at least see whether the working directory actually is $out/${attrs.ruby.gemPath}/gems/pg_query-${pgQueryVersion}/ext/pg_query
, but the patch is unable to find extconf.rb
.
I'm not familiar enough with Ruby and Nix to handle the |
@ariutta No, we should help you figure it out! I'll try to take a look this weekend. Thanks for getting this far on your own. |
When we get this working, we should close #24631 too. |
@ariutta I got it to build with ryantm@d3da316 and the binary it produces responds to help flags. Can you try testing that? If it works for you can you squash the commits down to one? I don't care if you try to keep my author credit or whatever. |
@ryantm yes, I think you've got it! There are a few differences between our Gemfile.lock and the original. Should they be the same? For example, should our Gemfile.lock list |
I think it should be the same! I had to change it to make the pg_query part work. |
It worked for me as well. Squash complete. |
@GrahamcOfBorg build sqlint |
Failure on aarch64-linux (full log) Attempted: sqlint Partial log (click to expand)
|
Success on x86_64-linux (full log) Attempted: sqlint Partial log (click to expand)
|
Success on x86_64-darwin (full log) Attempted: sqlint Partial log (click to expand)
|
Saw it failed on Can I call this? @GrahamcOfBorg build sqlint |
@GrahamcOfBorg build sqlint @ariutta You have to be trusted by the bot, because otherwise people could make PRs that run arbitrary code on it. |
No attempt on aarch64-linux (full log) The following builds were skipped because they don't evaluate on aarch64-linux: sqlint Partial log (click to expand)
|
Success on x86_64-linux (full log) Attempted: sqlint Partial log (click to expand)
|
Success on x86_64-darwin (full log) Attempted: sqlint Partial log (click to expand)
|
Motivation for this change
Add an SQL linter: https://github.com/purcell/sqlint
Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)