Skip to content

build: set remote origin url#1548

Merged
crazy-max merged 1 commit intodocker:masterfrom
crazy-max:git-ls-remote
Jan 26, 2023
Merged

build: set remote origin url#1548
crazy-max merged 1 commit intodocker:masterfrom
crazy-max:git-ls-remote

Conversation

@crazy-max
Copy link
Copy Markdown
Member

fixes #1540

By default we use the first remote url when setting vcs url. This change will use origin url. If it fails, fallback to upstream url. Otherwise get the first remote url.

Signed-off-by: CrazyMax crazy-max@users.noreply.github.com

Comment thread util/gitutil/gitutil_test.go
Copy link
Copy Markdown
Collaborator

@jedevc jedevc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, PTAL @tonistiigi

@crazy-max crazy-max force-pushed the git-ls-remote branch 3 times, most recently from f455b5e to ed91c32 Compare January 24, 2023 17:11
Comment thread util/gitutil/gitutil.go Outdated
if ru, err := c.clean(c.run("remote", "get-url", "upstream")); err == nil && ru != "" {
return ru, nil
}
// Otherwise try to get the remote URL from the first remote
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should just give up here. Using random remote gives random results.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there's only a single remote it makes sense that we should take that one IMO

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Give up like error out or skip setting vcs source in this case?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skip. Like we couldn't detect the origin.

Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
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.

[v0.10] vcs repo should send origin url, not first remote url

4 participants