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

Gracefully handle abbreviated revisions during import #987

Closed
carolynvs opened this issue Aug 9, 2017 · 6 comments · Fixed by #1027
Closed

Gracefully handle abbreviated revisions during import #987

carolynvs opened this issue Aug 9, 2017 · 6 comments · Fixed by #1027

Comments

@carolynvs
Copy link
Collaborator

carolynvs commented Aug 9, 2017

dep in general frowns on / doesn't support abbreviated revisions. However during import, and when reading external config on-the-fly, dep shouldn't just give up. Instead we should lookup the full revision for the short revision, and use the full revision in the imported lock, etc.

I'm hoping that we can handle this in SourceMgr.InferConstraint, so that all the importers get this for "free", though we probably need plumbing in gps to do the dirty work.

Notes:

  • We already have SourceMgr.RevisionPresentIn to check if the revision is valid, and for the git implementation the underlying command used actually prints the full revision, but nothing exists right now to bubble that up.
  • Currently InferConstraint doesn't actually hit the repository to figure out if a rev is valid, it just matches a pattern. Since we actually validate tags/branches, it may be time to retire the guesswork, like doing length checks, and just reply on the source manager entirely to check if a string is a valid revision (and get the full revision if it was abbreviated).
@sdboyer
Copy link
Member

sdboyer commented Aug 12, 2017

yeah, we'll definitely need some new plumbing.

i think trying to work it in via InferConstraint() is a wise place to start. i'd rather we not have to split it out into its own thing, but if we do, it's not the end of the world.

@markwest1
Copy link
Contributor

I've spoken with @carolynvs about this and she says I can give it a try.

@carolynvs
Copy link
Collaborator Author

I bequeath upon ye the sacride golden star of approval ⭐️

@spenczar
Copy link
Contributor

😟 Hope I didn't cut the line by submitting #1027 to fix this issue. Sorry for not checking in here first!

@carolynvs
Copy link
Collaborator Author

I think we all realized before much work was done. Since we can't easily assign issues in GH, it's a distributed game of Hungry Hungry Hippos. 😝 Sorry @markwest1!

@markwest1
Copy link
Contributor

No problem! I'm sure there's more work out there for me!

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

Successfully merging a pull request may close this issue.

4 participants