Skip to content
This repository has been archived by the owner on Mar 6, 2020. It is now read-only.

vendor: respect repository field in restore and update #730

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

cryptix
Copy link
Contributor

@cryptix cryptix commented Jan 10, 2018

Somehow the repository field from the manifest isn't used.

With this I can replace packages with forks.

@davecheney
Copy link
Contributor

davecheney commented Jan 10, 2018 via email

@cryptix
Copy link
Contributor Author

cryptix commented Jan 11, 2018

I deeply regret adding restore to gb vendor, ...

oh why is that?

I might not be best style but I tend to keep small patches inside my tree until I move them to PRs/external forks. It reduces the overhead of dealing with a lot more repos for me and the restore command is pretty helpful to get back the upstream state.

@cryptix cryptix changed the title respect repository field in restore and update vendor: respect repository field in restore and update Jan 11, 2018
@davecheney
Copy link
Contributor

davecheney commented Jan 11, 2018 via email

@cryptix
Copy link
Contributor Author

cryptix commented Jul 23, 2018

I just noticed an issue with my approach. Consider these two entries in manifest, they retrieve two packages from the same repository.

Since I just replaced importPath with repository, it now clones the root of the repositories into the two improtpath locations, leading to a broken vendor state.

Disregard the previous comment. Somehow my vendor/manifest didn't have "path": "..." entries to build the correct copy.

I broke the vanity-detection fallback with my previous commit.
This is just a workaround until there is a cleaner solution.
@codecov-io
Copy link

Codecov Report

Merging #730 into master will increase coverage by 0.18%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #730      +/-   ##
=========================================
+ Coverage   50.52%   50.7%   +0.18%     
=========================================
  Files          42      41       -1     
  Lines        3119    3102      -17     
=========================================
- Hits         1576    1573       -3     
+ Misses       1405    1386      -19     
- Partials      138     143       +5
Impacted Files Coverage Δ
build.go
cmd/gb/build.go 71.34% <0%> (+66.47%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f11784c...0153a33. Read the comment docs.

@cryptix
Copy link
Contributor Author

cryptix commented Jul 23, 2018

Testing this some more, I noticed that I broke vanity imports.

I pushed a really dirty hack in 0153a33 to make golang.org/x/... work again but it's not good since it only works for go.googlesource.com/....

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

Successfully merging this pull request may close these issues.

3 participants