cube2222 / octosql

OctoSQL is a query tool that allows you to join, analyse and transform data from multiple databases and file formats using SQL.
Mozilla Public License 2.0
4.75k stars 202 forks source link

Cannot install from source due to replace directive #282

Closed Fryuni closed 1 year ago

Fryuni commented 2 years ago

Installing a module-aware binary without modules has been blocked since Go 1.13 if I remember correctly.

❯ go install -u github.com/cube2222/octosql
flag provided but not defined: -u
usage: go install [build flags] [packages]
Run 'go help install' for details.
❯ go install github.com/cube2222/octosql    
no required module provides package github.com/cube2222/octosql; to add it:
        go get github.com/cube2222/octosql

The suggested command downloads the source but doesn't build it.

Replacing that with the new command using module installation (with a version) also does not work because replace directives are not supported with the install command.

❯ go install github.com/cube2222/octosql@latest
go: github.com/cube2222/octosql@latest (in github.com/cube2222/octosql@v0.8.0):
        The go.mod file for the module providing named packages contains one or
        more replace directives. It must not contain directives that would cause
        it to be interpreted differently than if it were the main module.
cube2222 commented 2 years ago

Thanks a lot for taking the time to report this!

I'll admit, I didn't know about this. I'll think about how to do the adaptations to the parquet library in a different way so that replacement isn't needed. No promises however - I'd like to avoid creating a full fork as that would require much more changes, and keeping up with the origin would be much harder.

In the meantime I recommend git cloning + installing or using the Homebrew core formula (which clones + builds behind the scenes).

Fryuni commented 2 years ago

Oh, don't worry. I just downloaded the pre-built binary from the release, I wasn't looking for anything unreleased

arikgrahl commented 2 years ago

I can confirm that the problem seems to exist since the integration of parquet, so since version 0.7.0.

go install github.com/cube2222/octosql@v0.7.0
go: downloading github.com/cube2222/octosql v0.7.0
go: github.com/cube2222/octosql@v0.7.0 (in github.com/cube2222/octosql@v0.7.0):
    The go.mod file for the module providing named packages contains one or
    more replace directives. It must not contain directives that would cause
    it to be interpreted differently than if it were the main module.

vs.

go install github.com/cube2222/octosql@v0.6.2
go: downloading github.com/cube2222/octosql v0.6.2
go: downloading github.com/Masterminds/semver v1.5.0
go: downloading github.com/skratchdot/open-golang v0.0.0-20200116055534-eef842397966
go: downloading github.com/spf13/cobra v1.2.1
go: downloading gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b
go: downloading github.com/pkg/errors v0.9.1
go: downloading github.com/valyala/fastjson v1.6.3
go: downloading github.com/jackc/pgx v3.6.2+incompatible
go: downloading github.com/google/btree v1.0.0
go: downloading github.com/oklog/ulid/v2 v2.0.2
go: downloading github.com/dgraph-io/ristretto v0.0.3
go: downloading github.com/awalterschulze/gographviz v2.0.3+incompatible
go: downloading github.com/mitchellh/go-homedir v1.0.0
go: downloading github.com/gosuri/uilive v0.0.4
go: downloading github.com/kr/text v0.2.0
go: downloading google.golang.org/grpc v1.42.0
go: downloading github.com/mholt/archiver v3.1.1+incompatible
go: downloading github.com/olekukonko/tablewriter v0.0.4
go: downloading github.com/segmentio/encoding v0.2.7
go: downloading github.com/spf13/pflag v1.0.5
go: downloading github.com/golang/protobuf v1.5.2
go: downloading google.golang.org/protobuf v1.27.1
go: downloading github.com/dsnet/compress v0.0.1
go: downloading github.com/golang/snappy v0.0.4
go: downloading github.com/nwaples/rardecode v1.1.2
go: downloading github.com/pierrec/lz4 v2.6.1+incompatible
go: downloading github.com/ulikunitz/xz v0.5.10
go: downloading github.com/xi2/xz v0.0.0-20171230120015-48954b6210f8
go: downloading github.com/mattn/go-runewidth v0.0.9
go: downloading golang.org/x/net v0.0.0-20211112202133-69e39bad7dc2
go: downloading google.golang.org/genproto v0.0.0-20211112145013-271947fe86fd
go: downloading golang.org/x/crypto v0.0.0-20201221181555-eec23a3978ad
go: downloading golang.org/x/text v0.3.7
go: downloading github.com/cespare/xxhash v1.1.0
go: downloading golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e

Unfortunately, this also breaks my Nix builds from source, on which I am dependent on NixOS, as I cannot use the pre-built binaries.

cat <<EOF | nix-build -
with import <nixpkgs> {};

buildGoModule rec {
  pname = "octosql";
  version = "0.6.2";

  src = fetchFromGitHub {
    owner  = "cube2222";
    repo   = pname;
    rev    = "v" + version;
    sha256 = "sha256-w+GQ/S02Y1OXnNNo/CPCRrMfPClMA+qKSdAKZx4UdNw=";
  };

  vendorSha256 = "sha256-PRCOExhHsdHKUDFswzewQ1oeRzjWzKiTIqkkxgmLhgY=";

  postInstall = ''
    rm -v \$out/bin/tester
  '';
}
EOF
/nix/store/jggglwp30jlwhb8y932gdfm17cfkzn5c-octosql-0.6.2

I can imagine that other platforms, for various reasons, do not want to use pre-built binaries as well.

Is there any way to support the recovery of the build process?

cube2222 commented 2 years ago

Would it be possible in the Nix package to instead git clone & go install?

It seems like it only breaks when go installing directly from a remote source control system (github in this case).

So in other words:

git clone github.com/cube2222/octosql
cd octosql
go install

It looks like that's also what the Homebrew core formula does: https://github.com/Homebrew/homebrew-core/blob/master/Formula/octosql.rb

There is also an issue in the golang repo tracking this problem: https://github.com/golang/go/issues/44840

arikgrahl commented 1 year ago

To put it simplifiedly, a git clone/go install is what the buildGoModule does, but in a reproducible and declarative way.

Nevertheless, this suggestion put me on the right track, for which I am grateful!

For the test definition of

buildPhase = ''
  go install
'';

I received multiple times the error message module requires Go 1.18. By using buildGo118Module instead of buildGoModule, I was able to successfully build the project.

Is Go >= 1.18 a requirement for the project? If so, this could possibly be documented.

For future reference, here is my Nix expression for other Nix users:

cat <<EOF | nix-build -
with import <nixpkgs> {};

buildGo118Module rec {
  pname = "octosql";
  version = "0.9.2";

  src = fetchFromGitHub {
    owner  = "cube2222";
    repo   = pname;
    rev    = "v" + version;
    sha256 = "sha256-f2RGAAjC+SmCVaegwbgE3UlvxlUNS3fJZQ6CWyS7/Gc=";
  };

  vendorSha256 = "sha256-ukNjLk1tTdw0bwXaYAEDuHfzxHuAX1xyqRqC6wmW/H4=";

  postInstall = ''
    rm -v \$out/bin/tester
  '';
}
EOF
cube2222 commented 1 year ago

Is Go >= 1.18 a requirement for the project? If so, this could possibly be documented.

Yes, good point! I've added it to the README.

Fryuni commented 1 year ago

@arikgrahl you can use patchelf to make pre-built binaries work with NixOS There are multiple examples on Nikpkgs itself.

Not the most convenient, but once you do it once it becomes very easy. It is faster since there is no need to rebuild everytime and sometimes better to report things since it is the distributed binary.

arikgrahl commented 1 year ago

Thank you for the advice, I am aware of patchelf. Nothing against providing pre-built binaries in general, but relying on them is for me unacceptable as I cannot audit the software completely and they are not portable enough, e.g. lack of arm (32 bit). To be honest, I don't understand your argument why pre-built binaries should be preferred regarding reporting.

As I stated above, I can successfully built the application with the given Nix expression, which takes less than 18 seconds on comodity hardware, including running the test suite. I will soon contribute my expression to nixpkgs. Once it is included it will be built as part of the Nix CI/CD infrastructure and a binary cache will be in place to further accelerate successive installations.

Fryuni commented 1 year ago

To be honest, I don't understand your argument why pre-built binaries should be preferred regarding reporting.

From my experience, due to how easy it is to install Go binaries from source, people install using @latest and then months later report a problem on an old version saying that it is using the latest version. So I normally recommend using the pre-built binaries, but not for any technical reason, it's more of a people reason.

Admittedly, since you are a Nix user I doubt this would apply to you.

arikgrahl commented 1 year ago

I will soon contribute my expression to nixpkgs. Once it is included it will be built as part of the Nix CI/CD infrastructure and a binary cache will be in place to further accelerate successive installations.

As also stated in PR https://github.com/cube2222/octosql/pull/294, my contribution of a Nix package of octosql to nixpkgs has successfully been accepted. Through this, the installation is straightforward and also includes a binary cache, so building from source should not be necessary.

The PR https://github.com/cube2222/octosql/pull/294 should also clarify the not working installation instructions, which are the essence of this issue.

From my point of view the issue can be closed by accepting the PR https://github.com/cube2222/octosql/pull/294.