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.

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

Merged
merged 1 commit into from Jun 18, 2018
Merged

sqlint: init at 0.1.7 #41901

merged 1 commit into from Jun 18, 2018

Conversation

ariutta
Copy link
Contributor

@ariutta ariutta commented Jun 12, 2018

Motivation for this change

Add an SQL linter: https://github.com/purcell/sqlint

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

Copy link
Member

@ryantm ryantm left a 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

@ariutta ariutta force-pushed the sqlint branch 2 times, most recently from 6947470 to 6214d18 Compare June 13, 2018 03:47
@@ -1489,6 +1489,8 @@ with pkgs;
utillinux = utillinuxMinimal;
};

sqlint = callPackage ../development/tools/sqlint { };
Copy link
Contributor Author

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?

Copy link
Member

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.

@ariutta
Copy link
Contributor Author

ariutta commented Jun 13, 2018

Squash complete.


gemConfig = {
pg_query = attrs: {
NIX_SSL_CERT_FILE = "${cacert}/etc/ssl/certs/ca-bundle.crt";
Copy link
Member

Choose a reason for hiding this comment

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

What is this for?

Copy link
Member

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.

Copy link
Contributor Author

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?

Copy link
Member

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.

Copy link
Contributor Author

@ariutta ariutta Jun 15, 2018

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.

Copy link
Contributor Author

@ariutta ariutta Jun 15, 2018

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

@Mic92
Copy link
Member

Mic92 commented Jun 13, 2018

@GrahamcOfBorg build sqlint

@GrahamcOfBorg
Copy link

Failure on aarch64-linux (full log)

Attempted: sqlint

Partial log (click to expand)


extconf failed, exit code 1

Gem files will remain installed in /nix/store/7q37m0kzkp083bn04qcc81rbikchwap8-ruby2.4.4-pg_query-0.13.5/lib/ruby/gems/2.4.0/gems/pg_query-0.13.5 for inspection.
Results logged to /nix/store/7q37m0kzkp083bn04qcc81rbikchwap8-ruby2.4.4-pg_query-0.13.5/lib/ruby/gems/2.4.0/extensions/aarch64-linux/2.4.0/pg_query-0.13.5/gem_make.out
builder for '/nix/store/q8fj6i9nq5crnspsyd1717iw888knzqm-ruby2.4.4-pg_query-0.13.5.drv' failed with exit code 1
cannot build derivation '/nix/store/wi29fbjqc3kgvfxijkm8j2gd3whl5ncn-ruby2.4.4-sqlint-0.1.7.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/5cdndbpfsy8avyyixvgd6fra5x4kb4rj-sqlint-0.1.7.drv': 2 dependencies couldn't be built
cannot build derivation '/nix/store/nwkxxnbmx9yqlidp0rdmif4ys2hr0v9p-sqlint-0.1.7.drv': 1 dependencies couldn't be built
error: build of '/nix/store/nwkxxnbmx9yqlidp0rdmif4ys2hr0v9p-sqlint-0.1.7.drv' failed

@GrahamcOfBorg
Copy link

Failure on x86_64-linux (full log)

Attempted: sqlint

Partial log (click to expand)


extconf failed, exit code 1

Gem files will remain installed in /nix/store/xnz9iic8v6mdbk8qp2rzy7scr11p1yxs-ruby2.4.4-pg_query-0.13.5/lib/ruby/gems/2.4.0/gems/pg_query-0.13.5 for inspection.
Results logged to /nix/store/xnz9iic8v6mdbk8qp2rzy7scr11p1yxs-ruby2.4.4-pg_query-0.13.5/lib/ruby/gems/2.4.0/extensions/x86_64-linux/2.4.0/pg_query-0.13.5/gem_make.out
builder for '/nix/store/pzxk5p2977yzkqa50cf6dkhkcvj9frfw-ruby2.4.4-pg_query-0.13.5.drv' failed with exit code 1
cannot build derivation '/nix/store/7wvvifm9747l600v4vqdkd3hn1pp4y18-ruby2.4.4-sqlint-0.1.7.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/pcxjpnqn58rxwhz6dz5gqf7lk9alb2g1-sqlint-0.1.7.drv': 2 dependencies couldn't be built
cannot build derivation '/nix/store/j5l18sc0wnw643d2b3y8aja9xnfcb3xw-sqlint-0.1.7.drv': 1 dependencies couldn't be built
error: build of '/nix/store/j5l18sc0wnw643d2b3y8aja9xnfcb3xw-sqlint-0.1.7.drv' failed

@GrahamcOfBorg
Copy link

Success on x86_64-darwin (full log)

Attempted: sqlint

Partial log (click to expand)

  Gem home: /nix/store/w3yqvxb7np7nimm9bd5djqhi5z21qpvm-ruby2.4.4-sqlint-0.1.7/lib/ruby/gems/2.4.0
Successfully installed sqlint-0.1.7
1 gem installed
post-installation fixup
patching script interpreter paths in /nix/store/w3yqvxb7np7nimm9bd5djqhi5z21qpvm-ruby2.4.4-sqlint-0.1.7
/nix/store/w3yqvxb7np7nimm9bd5djqhi5z21qpvm-ruby2.4.4-sqlint-0.1.7/lib/ruby/gems/2.4.0/gems/sqlint-0.1.7/bin/sqlint: interpreter directive changed from "/usr/bin/env ruby" to "/nix/store/0xzzx8zlj1486mvjjf85z15g985wjwzx-ruby-2.4.4/bin/ruby"
building '/nix/store/nk1b0mjamc0359lghf1xc99w6d405qhg-sqlint-0.1.7.drv'...
created 17 symlinks in user environment
building '/nix/store/h0y19gnv0c8kxx04117pg65jzaw4j1p5-sqlint-0.1.7.drv'...
/nix/store/hbvaz245wfcyzxlaqh6k4p4c6x4hcahm-sqlint-0.1.7

@ariutta
Copy link
Contributor Author

ariutta commented Jun 15, 2018

The linux build failures will likely be resolved if we can figure out how to download ahead of time and install the pg_query extension.

#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
Copy link
Contributor Author

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];
Copy link
Contributor Author

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.

@ariutta
Copy link
Contributor Author

ariutta commented Jun 15, 2018

I'm not familiar enough with Ruby and Nix to handle the libpg-query extension. Should I close this pull request?

@ryantm
Copy link
Member

ryantm commented Jun 16, 2018

@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.

@ryantm
Copy link
Member

ryantm commented Jun 16, 2018

When we get this working, we should close #24631 too.

@ryantm
Copy link
Member

ryantm commented Jun 16, 2018

@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.

@ariutta
Copy link
Contributor Author

ariutta commented Jun 16, 2018

@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 sqlint as a dependency when the original doesn't?

@ryantm
Copy link
Member

ryantm commented Jun 16, 2018

I think it should be the same! I had to change it to make the pg_query part work.

@ariutta
Copy link
Contributor Author

ariutta commented Jun 16, 2018

It worked for me as well. Squash complete.

@ryantm
Copy link
Member

ryantm commented Jun 17, 2018

@GrahamcOfBorg build sqlint

@GrahamcOfBorg
Copy link

Failure on aarch64-linux (full log)

Attempted: sqlint

Partial log (click to expand)


make failed, exit code 2

Gem files will remain installed in /nix/store/sv05pi4pnlh3j13x7p1vb91p07fik06d-ruby2.4.4-pg_query-1.0.0/lib/ruby/gems/2.4.0/gems/pg_query-1.0.0 for inspection.
Results logged to /nix/store/sv05pi4pnlh3j13x7p1vb91p07fik06d-ruby2.4.4-pg_query-1.0.0/lib/ruby/gems/2.4.0/extensions/aarch64-linux/2.4.0/pg_query-1.0.0/gem_make.out
builder for '/nix/store/x2frihqqnc3y3xsv3ywixl00qsnq9y9l-ruby2.4.4-pg_query-1.0.0.drv' failed with exit code 1
cannot build derivation '/nix/store/7j9qzk60mmck6dabkhgipnsad552z21m-ruby2.4.4-sqlint-0.1.7.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/xjcjqygr8p5h333vndqv4x469hd6yny3-sqlint-0.1.7.drv': 2 dependencies couldn't be built
cannot build derivation '/nix/store/grw3rqikl6b49cbyysy611ks14xc2299-sqlint-0.1.7.drv': 1 dependencies couldn't be built
error: build of '/nix/store/grw3rqikl6b49cbyysy611ks14xc2299-sqlint-0.1.7.drv' failed

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: sqlint

Partial log (click to expand)

1 gem installed
post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/zl3mm0vagq57bylz02rspl0yssymf327-ruby2.4.4-sqlint-0.1.7
patching script interpreter paths in /nix/store/zl3mm0vagq57bylz02rspl0yssymf327-ruby2.4.4-sqlint-0.1.7
/nix/store/zl3mm0vagq57bylz02rspl0yssymf327-ruby2.4.4-sqlint-0.1.7/lib/ruby/gems/2.4.0/gems/sqlint-0.1.7/bin/sqlint: interpreter directive changed from "/usr/bin/env ruby" to "/nix/store/x6xs3dnvfa7739x3ij04f83fjzb3d587-ruby-2.4.4/bin/ruby"
checking for references to /build in /nix/store/zl3mm0vagq57bylz02rspl0yssymf327-ruby2.4.4-sqlint-0.1.7...
building '/nix/store/sjmkh7m0d3mrl8z6p5h68sbs94y25crg-sqlint-0.1.7.drv'...
created 17 symlinks in user environment
building '/nix/store/jgn9nl0n6ax9mzcbs9c39g4kqxbjjrnc-sqlint-0.1.7.drv'...
/nix/store/1p63l4nwnh1bkgpx5awjz3nak33f35xb-sqlint-0.1.7

@GrahamcOfBorg
Copy link

Success on x86_64-darwin (full log)

Attempted: sqlint

Partial log (click to expand)

  Gem home: /nix/store/gac675jqcch5g3dxxyj4awia8hbzqcwd-ruby2.4.4-sqlint-0.1.7/lib/ruby/gems/2.4.0
Successfully installed sqlint-0.1.7
1 gem installed
post-installation fixup
patching script interpreter paths in /nix/store/gac675jqcch5g3dxxyj4awia8hbzqcwd-ruby2.4.4-sqlint-0.1.7
/nix/store/gac675jqcch5g3dxxyj4awia8hbzqcwd-ruby2.4.4-sqlint-0.1.7/lib/ruby/gems/2.4.0/gems/sqlint-0.1.7/bin/sqlint: interpreter directive changed from "/usr/bin/env ruby" to "/nix/store/vywghxz5vzvm84625mi21z556dz936dp-ruby-2.4.4/bin/ruby"
building '/nix/store/pmbv6b9vaj6bpn6awsqpq6j1zh5mckb3-sqlint-0.1.7.drv'...
created 17 symlinks in user environment
building '/nix/store/sm9s295mp8crh5ml0z80lhpx8gq95355-sqlint-0.1.7.drv'...
/nix/store/gza7rkkfdm7pbx15sbnr3msk20kcsjcw-sqlint-0.1.7

@ariutta
Copy link
Contributor Author

ariutta commented Jun 17, 2018

Saw it failed on aarch64-linux. I have no idea how to fix that, so I limited this package to x86_64-linux and x86_64-darwin and squashed again.

Can I call this?

@GrahamcOfBorg build sqlint

@ryantm
Copy link
Member

ryantm commented Jun 17, 2018

@GrahamcOfBorg build sqlint

@ariutta You have to be trusted by the bot, because otherwise people could make PRs that run arbitrary code on it.

@GrahamcOfBorg
Copy link

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)


a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowUnsupportedSystem = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowUnsupportedSystem = true; }
to ~/.config/nixpkgs/config.nix.


@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: sqlint

Partial log (click to expand)

these derivations will be built:
  /nix/store/g5zs7hk4jplq7r2r8fnwh0bnb1pd4xfw-sqlint-0.1.7.drv
building '/nix/store/g5zs7hk4jplq7r2r8fnwh0bnb1pd4xfw-sqlint-0.1.7.drv'...
/nix/store/fw014kahmwfpxjfc7rsfl7fd3dgzrix2-sqlint-0.1.7

@GrahamcOfBorg
Copy link

Success on x86_64-darwin (full log)

Attempted: sqlint

Partial log (click to expand)

these derivations will be built:
  /nix/store/gw2l4f025mwgvaf597n1d9rx97nwnlhp-sqlint-0.1.7.drv
building '/nix/store/gw2l4f025mwgvaf597n1d9rx97nwnlhp-sqlint-0.1.7.drv'...
/nix/store/f05qcwn6lsj22gfdn2h0p5xgb24hpyqw-sqlint-0.1.7

@Mic92 Mic92 merged commit a52d233 into NixOS:master Jun 18, 2018
@xeji xeji mentioned this pull request Jul 11, 2018
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants