thought-machine / please

High-performance extensible build system for reproducible multi-language builds.
https://please.build
Apache License 2.0
2.47k stars 206 forks source link

Libraries with vendored dependencies... #445

Closed sean- closed 6 years ago

sean- commented 6 years ago

I'm running into a strange quirk. I need to build and use the nomad API. Mirroring something similar to what dep had produced, I have the following:

go_get(
     name = "nomad",
     get = "github.com/hashicorp/nomad",
     binary = False,
     install = [
         "acl",
         "api",
         "api/contexts",
         "helper",
         "helper/args",
         "helper/discover",
         "helper/flatmap",
         "helper/uuid",
         "jobspec",
         "nomad/structs",
     ],
     revision = "e6e7966469817d807849757d90088c95a51b3e77",
 )

I'm able to build this package, however when it gets to link time in my application, I'm getting the following linking error:

% plz build
Build stopped after 10.37s. 1 target failed:
    //cmd/mycmd:mycmd
Error building target //cmd/dcpm:dcpm: exit status 1
/Users/seanc/go1.11/pkg/tool/darwin_amd64/link: cannot open file /Users/seanc/go1.11/pkg/darwin_amd64/github.com/hashicorp/nomad/vendor/github.com/hashicorp/go-cleanhttp.a: open /Users/seanc/go1.11/pkg/darwin_amd64/github.com/hashicorp/nomad/vendor/github.com/hashicorp/go-cleanhttp.a: no such file or directory

Is there something that I'm supposed to do pull in the upstream vendored library? In my local repository, I'm also using github.com/hashicorp/go-cleanhttp but a different SHA than what's vendoreed in nomad. What should I do in plz? Right now this isn't entirely obvious to me. What I'm inclined, but somewhat loathe to do, is add strip = ["vendor/"], to my go_get() for the nomad target, and then add explicit deps for each of the following:

         "github.com/hashicorp/go-cleanhttp",
         "github.com/hashicorp/nomad/vendor/github.com/hashicorp/golang-lru",
         "github.com/hashicorp/nomad/vendor/github.com/hashicorp/raft",
         "github.com/hashicorp/nomad/vendor/github.com/hashicorp/consul/api",
         "github.com/hashicorp/nomad/vendor/github.com/hashicorp/go-version",
         "github.com/hashicorp/nomad/vendor/github.com/mitchellh/copystructure",
         "github.com/hashicorp/nomad/vendor/github.com/ugorji/go/codec",
         "github.com/hashicorp/nomad/vendor/github.com/hashicorp/go-msgpack/codec",
         "github.com/hashicorp/nomad/vendor/github.com/hashicorp/errwrap",
         "github.com/hashicorp/nomad/vendor/github.com/hashicorp/hcl/hcl/token",
         "github.com/hashicorp/nomad/vendor/github.com/hashicorp/golang-lru/simplelru",
         "github.com/hashicorp/nomad/vendor/github.com/hashicorp/go-immutable-radix",
         "github.com/hashicorp/nomad/vendor/github.com/hashicorp/hcl",
         "github.com/hashicorp/nomad/vendor/github.com/armon/go-metrics",
         "github.com/hashicorp/nomad/vendor/github.com/gorhill/cronexpr",
         "github.com/hashicorp/nomad/vendor/github.com/hashicorp/go-cleanhttp",
         "github.com/hashicorp/nomad/vendor/github.com/hashicorp/go-multierror",
         "github.com/hashicorp/nomad/vendor/github.com/hashicorp/go-rootcerts",
         "github.com/hashicorp/nomad/vendor/github.com/hashicorp/hcl/hcl/ast",
         "github.com/hashicorp/nomad/vendor/github.com/mitchellh/go-homedir",
         "github.com/hashicorp/nomad/vendor/github.com/mitchellh/hashstructure",
         "github.com/hashicorp/nomad/vendor/golang.org/x/crypto/blake2b",

which is a bit of a bear, especially when you factor in their transitive dependencies. It feels like I'm missing something here and this should "just work," but maybe not and I do have to do this? At that point, I'm taking ownership of the library maintenance and updating my libraries to match everything across all versions, which is more correct, but also without any tooling to update the revisions in all of the BUILD files, is a a huge maintenance burden. ? Thanks in advance.

peterebden commented 6 years ago

Agreed these are awkward. go get is not a great help about it unfortunately; it does enough that we sort of need to lean on some of its logic (so as not to have to reinvent it all ourselves) but it's not very flexible about things like this (for example it would be a lot easier to be able to use /... in some places, but it tends to then complain about packages with no non-test files and things...).

I'm of course open to any improvements to the logic of it, but may be useful to have some tooling (a little like Gazelle?) to help create these.

ryancurrah commented 6 years ago

I'm sorry if this is not right issue. I am having the same error building target trying to setup my dependencies.

plz build //twitducer:twitducer
Build stopped after 1.28s. 1 target failed:
    //twitducer:twitducer
Error building target //twitducer:twitducer: exit status 1
/usr/local/Cellar/go/1.11.2/libexec/pkg/tool/darwin_amd64/link: cannot open file /usr/local/Cellar/go/1.11.2/libexec/pkg/darwin_amd64/github.com/ChimeraCoder/anaconda/vendor/github.com/azr/backoff.a: open /usr/local/Cellar/go/1.11.2/libexec/pkg/darwin_amd64/github.com/ChimeraCoder/anaconda/vendor/github.com/azr/backoff.a: no such file or directory
sean- commented 6 years ago

Well, we did the regrettable thing and forked Nomad's vendored dependencies and have taken ownership of its transitive dependencies.

go_get(
    name = "nomad",
    get = "github.com/hashicorp/nomad",
    repo = "github.com/hashicorp/nomad",
    binary = False,
    install = [
        "acl",
        "api",
        "api/contexts",
        "helper",
        "helper/args",
        "helper/discover",
        "helper/flatmap",
        "helper/uuid",
        "jobspec",
        "nomad/structs",
    ],
    deps = [
        ":consul",
        ":copystructure",
        ":cronexpr",
        ":go-cleanhttp",
        ":go-immutable-radix",
        ":go-multierror",
        ":go-rootcerts",
        ":go-version",
        ":hashstructure",
        ":hcl",
        ":mapstructure",
        ":raft",
        ":ugorji-go-codec",
        ":x-crypto",
    ],
    strip = ["vendor/"],
    revision = "e6e7966469817d807849757d90088c95a51b3e77",
)

go_get(
    name = "consul",
    get = "github.com/hashicorp/consul",
    repo = "github.com/hashicorp/consul",
    binary = False,
    install = [
        "api",
        "lib/freeport",
    ],
    deps = [
        ":go-cleanhttp",
        ":go-testing-interface",
        ":go-rootcerts",
        ":mapstructure",
        ":serf",
    ],
    strip = ["vendor/"],
    revision = "f22f6f992472f215d7d00975c4a9955dd9366cf7",
)

go_get(
    name = "go-cleanhttp",
    get = "github.com/hashicorp/go-cleanhttp",
    revision = "d5fe4b57a186c716b0e00b8c301cbd9b4182694d",
)

go_get(
    name = "go-testing-interface",
    get = "github.com/mitchellh/go-testing-interface",
    revision = "6d0b8010fcc857872e42fc6c931227569016843c",
)

go_get(
    name = "go-rootcerts",
    get = "github.com/hashicorp/go-rootcerts",
    deps = [
        ":go-homedir",
    ],
    revision = "6bb64b370b90e7ef1fa532be9e591a81c3493e00",
)

go_get(
    name = "go-homedir",
    get = "github.com/mitchellh/go-homedir",
    revision = "3864e76763d94a6df2f9960b16a20a33da9f9a66",
)

go_get(
    name = "mapstructure",
    get = "github.com/mitchellh/mapstructure",
    revision = "f15292f7a699fcc1a38a80977f80a046874ba8ac",
)

go_get(
    name = "serf",
    get = "github.com/hashicorp/serf",
    revision = "75714e08b2cf6cc7409a3f5a49e20293528acf69",
    deps = [
        ":go-metrics",
    ],
    install = [
        "coordinate",
    ],
    strip = ["vendor/"],
)

# ... etc.  All done by hand.

It works, but is a sub-optimal user experience. That said, it also daylighted all of the implicit magic that we take for granted in either dep or go get that was... there in plain sight, but never made explicit. For now, we will continue to do this as we are very, very pleased with the results (no pun intended, I promise).

sean- commented 6 years ago

The important note for future readers is we strip = ["vendor/"]'ed out all of the dependencies that come from a given package and then we have to manually traverse their Gopkg.lock dependencies. ... or in some cases, use a new SHA and assume the risk of the newly blended software. In the common case this has worked out okay (though we do have a few .patch files that we carry around for a few of our dependencies). Hurray for OSS and non-explicit forking! ... or something. ;)