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.

Sign up for GitHub

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

Jump to bottom

go: failure to handle an import path replacement directive #13138

Closed
tdyas opened this issue Oct 6, 2021 · 2 comments
Closed

go: failure to handle an import path replacement directive #13138

tdyas opened this issue Oct 6, 2021 · 2 comments
Labels
backend: Go Go backend-related issues

Comments

@tdyas
Copy link
Contributor

tdyas commented Oct 6, 2021 •
edited
Loading

Context: I am trying to get external-dns to build with the Pants Go plugin on this branch.

I encountered this following error while running ./pants_from_sources package :::

Exception message: 1 Exception encountered:

  ProcessExecutionFailure: Process 'Determine metadata for Go external module github.com/hashicorp/consul/api@v1.3.0' failed with exit code 1.
stdout:

stderr:
go: github.com/hashicorp/consul/sdk@v0.3.0 (replaced by ../sdk): reading ../sdk/go.mod: open /private/var/folders/md/0q71p41n0rbgwhnc8npjjzg00000gn/T/sdk/go.mod: no such file or directory
go: downloading github.com/mitchellh/mapstructure v1.1.2
go: downloading github.com/hashicorp/go-rootcerts v1.0.0
go: downloading github.com/hashicorp/serf v0.8.2
go: downloading github.com/hashicorp/go-cleanhttp v0.5.1
go: github.com/hashicorp/consul/sdk@v0.3.0 (replaced by ../sdk): reading ../sdk/go.mod: open /private/var/folders/md/0q71p41n0rbgwhnc8npjjzg00000gn/T/sdk/go.mod: no such file or directory



Use --no-process-execution-local-cleanup to preserve process chroots for inspection.

The import path replacement comes from this line in the github.com/hashicorp/consul/api module: https://github.com/hashicorp/consul/blob/2d05164be1d30fc6ec2ad0126294e009001e400b/api/go.mod#L5

which directs Go to replace import paths "github.com/hashicorp/consul/sdk" with a reference to a directory in the same repository (../sdk).
@tdyas tdyas added the backend: Go Go backend-related issues label Oct 6, 2021
@tdyas tdyas changed the title go: failure to handle a import path replacement directive go: failure to handle an import path replacement directive Oct 6, 2021
@tdyas

This comment has been minimized.

Sign in to view
@Eric-Arellano Eric-Arellano mentioned this issue Oct 25, 2021
Closed
Eric-Arellano added a commit to Eric-Arellano/pants that referenced this issue Oct 27, 2021
@Eric-Arellano
Add regression test for pantsbuild#13138
5271ea6 
# Rust tests and lints will be skipped. Delete if not intended.
[ci skip-rust]

# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
@Eric-Arellano Eric-Arellano mentioned this issue Oct 27, 2021
Merged
Eric-Arellano added a commit that referenced this issue Oct 27, 2021
@Eric-Arellano
[internal] Don't download Go third-party dependencies multiple times (#…
07a308e 
…13352)

Turns out that #13339 didn't actually work - we are redownloading the same modules several times with Go! Downloads happen when:

1. determining `GoModInfo` (once per `go.mod`)
2. `AllDownloadedModules` (once per `go.mod`)
3.  Determining metadata for each third-party package (once per third-party module)
4. Determining metadata for each first-party package (once per first-party package/directory)

This PR fixes it so that we only download modules a single time, once per `go.mod`.

To fix this, we stop treating third-party modules like first-party modules, i.e. we stop `cd`-ing into its downloaded directory and running `go list` directly in it, using its own `go.mod` and `go.sum`. That requires that the chroot has all of the module's transitive dependencies present, and it also resulted in issues like #13138. Instead, the much simpler thing to do is run `go list '...'` to do all third-party analysis in a single swoop. That gets us all the analysis we need.

We also extract the relevant `.go` files from all of the downloaded `GOPATH`, i.e. all the downloaded modules. For compilation, all we need is the `.go` files + the metadata we had earlier calculated. Compilation doesn't need access to anything else like other package's.

For first-party analysis, we copy the whole `GOPATH` into the chroot. (This is really slow! We need something like #12716 to fix this.)

## Benchmark

Running in https://github.com/toolchainlabs/remote-api-tools.

Before:

```
❯ hyperfine -r 5 './pants_from_sources --no-process-execution-local-cache --no-pantsd package ::'
  Time (mean ± σ):     36.467 s ±  0.603 s    [User: 41.109 s, System: 38.095 s]
  Range (min … max):   35.518 s … 37.137 s    5 runs
```

Fixing only third-party analysis:

```
❯ hyperfine -r 5 --show-output './pants_from_sources --no-process-execution-local-cache --no-pantsd package ::'
  Time (mean ± σ):     29.880 s ±  0.901 s    [User: 29.564 s, System: 15.281 s]
  Range (min … max):   28.835 s … 31.312 s    5 runs
```

Fixing everything:

```
❯ hyperfine -r 5 './pants_from_sources --no-process-execution-local-cache --no-pantsd package ::'
  Time (mean ± σ):     26.633 s ±  2.283 s    [User: 24.115 s, System: 30.453 s]
  Range (min … max):   24.570 s … 30.037 s    5 runs
```

[ci skip-rust]
[ci skip-build-wheels]
@Eric-Arellano
Copy link
Contributor

Closed by #13352

Eric-Arellano added a commit to Eric-Arellano/pants that referenced this issue Oct 27, 2021
@Eric-Arellano
[internal] Don't download Go third-party dependencies multiple times (p…
2f8dc61 
…antsbuild#13352)

Turns out that pantsbuild#13339 didn't actually work - we are redownloading the same modules several times with Go! Downloads happen when:

1. determining `GoModInfo` (once per `go.mod`)
2. `AllDownloadedModules` (once per `go.mod`)
3.  Determining metadata for each third-party package (once per third-party module)
4. Determining metadata for each first-party package (once per first-party package/directory)

This PR fixes it so that we only download modules a single time, once per `go.mod`.

To fix this, we stop treating third-party modules like first-party modules, i.e. we stop `cd`-ing into its downloaded directory and running `go list` directly in it, using its own `go.mod` and `go.sum`. That requires that the chroot has all of the module's transitive dependencies present, and it also resulted in issues like pantsbuild#13138. Instead, the much simpler thing to do is run `go list '...'` to do all third-party analysis in a single swoop. That gets us all the analysis we need.

We also extract the relevant `.go` files from all of the downloaded `GOPATH`, i.e. all the downloaded modules. For compilation, all we need is the `.go` files + the metadata we had earlier calculated. Compilation doesn't need access to anything else like other package's.

For first-party analysis, we copy the whole `GOPATH` into the chroot. (This is really slow! We need something like pantsbuild#12716 to fix this.)

## Benchmark

Running in https://github.com/toolchainlabs/remote-api-tools.

Before:

```
❯ hyperfine -r 5 './pants_from_sources --no-process-execution-local-cache --no-pantsd package ::'
  Time (mean ± σ):     36.467 s ±  0.603 s    [User: 41.109 s, System: 38.095 s]
  Range (min … max):   35.518 s … 37.137 s    5 runs
```

Fixing only third-party analysis:

```
❯ hyperfine -r 5 --show-output './pants_from_sources --no-process-execution-local-cache --no-pantsd package ::'
  Time (mean ± σ):     29.880 s ±  0.901 s    [User: 29.564 s, System: 15.281 s]
  Range (min … max):   28.835 s … 31.312 s    5 runs
```

Fixing everything:

```
❯ hyperfine -r 5 './pants_from_sources --no-process-execution-local-cache --no-pantsd package ::'
  Time (mean ± σ):     26.633 s ±  2.283 s    [User: 24.115 s, System: 30.453 s]
  Range (min … max):   24.570 s … 30.037 s    5 runs
```

[ci skip-rust]
[ci skip-build-wheels]
@Eric-Arellano Eric-Arellano mentioned this issue Oct 27, 2021
Merged
Eric-Arellano added a commit that referenced this issue Oct 27, 2021
@Eric-Arellano
[internal] Don't download Go third-party dependencies multiple times …
56c575a 
…(Cherry-pick of #13352) (#13378)

Turns out that #13339 didn't actually work - we are redownloading the same modules several times with Go! Downloads happen when:

1. determining `GoModInfo` (once per `go.mod`)
2. `AllDownloadedModules` (once per `go.mod`)
3.  Determining metadata for each third-party package (once per third-party module)
4. Determining metadata for each first-party package (once per first-party package/directory)

This PR fixes it so that we only download modules a single time, once per `go.mod`.

To fix this, we stop treating third-party modules like first-party modules, i.e. we stop `cd`-ing into its downloaded directory and running `go list` directly in it, using its own `go.mod` and `go.sum`. That requires that the chroot has all of the module's transitive dependencies present, and it also resulted in issues like #13138. Instead, the much simpler thing to do is run `go list '...'` to do all third-party analysis in a single swoop. That gets us all the analysis we need.

We also extract the relevant `.go` files from all of the downloaded `GOPATH`, i.e. all the downloaded modules. For compilation, all we need is the `.go` files + the metadata we had earlier calculated. Compilation doesn't need access to anything else like other package's.

For first-party analysis, we copy the whole `GOPATH` into the chroot. (This is really slow! We need something like #12716 to fix this.)

## Benchmark

Running in https://github.com/toolchainlabs/remote-api-tools.

Before:

```
❯ hyperfine -r 5 './pants_from_sources --no-process-execution-local-cache --no-pantsd package ::'
  Time (mean ± σ):     36.467 s ±  0.603 s    [User: 41.109 s, System: 38.095 s]
  Range (min … max):   35.518 s … 37.137 s    5 runs
```

Fixing only third-party analysis:

```
❯ hyperfine -r 5 --show-output './pants_from_sources --no-process-execution-local-cache --no-pantsd package ::'
  Time (mean ± σ):     29.880 s ±  0.901 s    [User: 29.564 s, System: 15.281 s]
  Range (min … max):   28.835 s … 31.312 s    5 runs
```

Fixing everything:

```
❯ hyperfine -r 5 './pants_from_sources --no-process-execution-local-cache --no-pantsd package ::'
  Time (mean ± σ):     26.633 s ±  2.283 s    [User: 24.115 s, System: 30.453 s]
  Range (min … max):   24.570 s … 30.037 s    5 runs
```

[ci skip-rust]
[ci skip-build-wheels]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
backend: Go Go backend-related issues
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[wip] Fix Go not building third-party modules with a replace directive Eric-Arellano/pants
2 participants
@tdyas @Eric-Arellano