automerge / automerge-go

MIT License
94 stars 6 forks source link

Can't use with go mod vendor #14

Closed trapped closed 1 year ago

trapped commented 1 year ago

Because of https://github.com/golang/go/issues/26366#issuecomment-405683150, go mod vendor only keeps files from the same-level directory as the installed package.

When vendoring automerge-go, this is what we're getting:

> tree ./vendor/github.com/automerge/
./vendor/github.com/automerge/
└── automerge-go
    ├── LICENSE.MIT
    ├── README.md
    ├── automerge.go
    ├── automerge.h
    ├── changes.go
    ├── counter.go
    ├── doc.go
    ├── item.go
    ├── list.go
    ├── map.go
    ├── normalize.go
    ├── path.go
    ├── result.go
    ├── sync_state.go
    ├── text.go
    ├── tools.go
    └── value.go

1 directory, 17 files

This then fails building with:

/Users/user/.gvm/gos/go1.20/pkg/tool/darwin_arm64/link: running clang failed: exit status 1
ld: warning: directory not found for option '-L/Users/user/automerge-test/vendor/github.com/automerge/automerge-go/deps/darwin_arm64'
ld: library not found for -lautomerge_core
clang: error: linker command failed with exit code 1 (use -v to see invocation)

Not using go mod vendor fixes the issue.

Perhaps the deps/ subdirectory contents could be lifted one level up?

ConradIrwin commented 1 year ago

Thanks for the report, and perfect timing – I'm about to rebuild all the binaries anyway.

I think the solution you describe of moving everything into the top level directory would work. It displeases my aesthetic sense, but it seems easier than trying to workaround it with sub-packages.

trapped commented 1 year ago

Thanks for the quick answer too! I too find it a bit cluttered. It appears the SQLite bindings (sqlite3-binding.c) also do the same though.

ConradIrwin commented 1 year ago

Does the version here: https://github.com/automerge/automerge-go/pull/13 work for you?

trapped commented 1 year ago

@ConradIrwin It does:

> go get github.com/automerge/automerge-go@085490d854057daca1331119d66277fbcf60dbab
> go mod vendor
> tree vendor/github.com/automerge/automerge-go/
vendor/github.com/automerge/automerge-go/
├── LICENSE.MIT
├── README.md
├── automerge.go
├── automerge.h
├── changes.go
├── counter.go
├── deps
│   ├── deps.go
│   ├── libautomerge_core_darwin_amd64.a
│   ├── libautomerge_core_darwin_arm64.a
│   ├── libautomerge_core_linux_amd64.a
│   ├── libautomerge_core_linux_arm64.a
│   ├── rebuild.sh
│   └── test_all.sh
├── doc.go
├── item.go
├── list.go
├── map.go
├── normalize.go
├── path.go
├── result.go
├── sync_state.go
├── text.go
├── tools.go
└── value.go

1 directory, 24 files

Very cool to see from the PR that it's actually enough to turn deps/ into a Go package. Thanks for the quick fix 🙂

ConradIrwin commented 1 year ago

Glad to hear! Thanks for the report, vendoring isn't something I'd tested with.

Out of curiosity, what leads you to use vendor directories? (I'm working on proposing a change to how go run works, so trying to understand more how other people's go projects are set up).

trapped commented 1 year ago

Mostly we need to maintain some legacy projects that still use vendoring; they have a few build scripts built around using the vendor/ directory as a dependency cache.

Often the actual go build is run inside a Docker container using a "builder" image pre-packaged with vetted/pinned tool versions and configs (such as golangci-lint); the Docker container is run without credentials so it wouldn't be able to pull private dependencies in.