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

fix: handle projects using Go module vendoring #1668

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

davidjb
Copy link

@davidjb davidjb commented Mar 7, 2023

Description

For Go projects, the call to install github.com/icarus-sullivan/mock-lambda in GoRunner.js fails in a Go project that is using vendoring
(https://go.dev/ref/mod#vendoring) because the subsequent go build command will error like so:

go: inconsistent vendoring in /path/dev/nimble:
    github.com/icarus-sullivan/mock-lambda@v0.0.0-20220115083805-e065469e964a: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt

    To ignore the vendor directory, use -mod=readonly or -mod=mod.
    To sync the vendor directory, run:
            go mod vendor

In the current codebase, this error gets swallowed and passes silently, until causing a failure to call the non-existent ./tmp executable with the call to execa(./tmp):

Error: Command failed with ENOENT: ./tmp  spawn ./tmp ENOENT

This PR detects if vendor/ is present in the project and then will re-run go mod vendor after the installation of mock-lambda to ensure that go build can succeed.

This change also raises any exceptions that occur within either of these install/build commands so it is obvious what problem is occurring. With this PR, the above error for go mod vendor is outputted instead at the time of the error occurring rather than indirectly causing a problem later (as are any other errors that may have occurred in the go build process).

Motivation and Context

Go projects using vendoring cannot currently be used with serverless-offline because the above issue. As mentioned, errors are also being obscured due to how the current GoRunner building code is ignoring exceptions. This PR addresses both issues.

How Has This Been Tested?

Tested locally with a vendored Go project and then again after removing the vendor directory; the handler continues to work as expected in both scenarios. Existing unit tests are passing.

Screenshots (if appropriate):

The call to install `github.com/icarus-sullivan/mock-lambda` will fail
in a Go project that is using vendoring
(https://go.dev/ref/mod#vendoring) as the subsequent `go build` command
will error like so:

    go: inconsistent vendoring in /path/dev/project:
        github.com/icarus-sullivan/mock-lambda@v0.0.0-20220115083805-e065469e964a: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt

        To ignore the vendor directory, use -mod=readonly or -mod=mod.
        To sync the vendor directory, run:
                go mod vendor

This change detects if `vendor/` is present and then will re-run
`go mod vendor` after the installation of `mock-lambda` to ensure that
building succeeds.

This also raises any exceptions that occur within either of these
install/build commands so it is obvious what problem is occurring.
Previously, this error was swallowed and passed silently, until causing
a failure to call the non-existent `./tmp`
executable with `execa(`./tmp`)`:

    Error: Command failed with ENOENT: ./tmp  spawn ./tmp ENOENT

With this PR, the above error for `go mod vendor` is outputted instead.
@DorianMazur
Copy link
Collaborator

DorianMazur commented Apr 30, 2024

Hey @davidjb Can you create tests that will test if go module vendoring works correctly?

@davidjb
Copy link
Author

davidjb commented Apr 30, 2024

@DorianMazur Since opening the PR, I’ve moved away from using Serverless due to it lacking support for recent AWS changes. Anyone’s free to add to this PR though 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants