-
Notifications
You must be signed in to change notification settings - Fork 16
umpf: add built-in support for synchronizing topic branches #53
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
base: master
Are you sure you want to change the base?
Conversation
4353753
to
b48609e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like this in general. A lot of small stuff, and as noted below, I think you can remove the local special case.
umpf
Outdated
fi | ||
local rtagrev="$(resolve_tag "${remote}" ${tagname})" | ||
local rtagrevf="$(resolve_commitish ${rtagrev}^)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use resolve_tag
here as well to make it less confusing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using rev-parse on FETCH_HEAD^
as resolve_remote_tag uses ls-remote which can't deal with ^
umpf
Outdated
commit="refs/tags/$tag" | ||
fi | ||
resolve_commitish "${commit}^{}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is only used to compare the local and remote tags. What's the use-case for accepting different tags with the same commit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure, it's dropped now though.
umpf
Outdated
if [ -n "${remote}" ]; then | ||
# Needed, so git rev-parse below can check for existent branches | ||
git fetch --quiet --no-tags ${remote} 2>/dev/null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, use ls-remote
for the branch checks instead.
That would make a --force-with-lease
possible as well (I'd like to see that as well at some point but I won't insist on it for this PR).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Takes way too long to use ls-remote on each of my 51 topic branches :/
I kept the fetch, but now use ls-remote to check for tag existence.
Also for comparing if the remote tag and the local one are equal I need to check the previous commit to the tag (the flattened history) to compare, but I can't do that with ls-remote, can I?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't call ls-remote for each branch. Call it once and create a map with branch name as key and hash as value. That should not take longer than any single fetch.
Why do you need to compare the previous commit? Having the same tag for different commits locally and remote is a really bad idea.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you need to compare the previous commit? Having the same tag for different commits locally and remote is a really bad idea.
It happens all the time, developer A tags a new umpf, tries to push it and finds that it has been already pushed and they should've fetched beforehand. Especially with multiple remotes for different customers.
I thus want some safety measure to verify that the remote tag is indeed corresponding to what I have locally. Unfortunately, what I have locally might only be an umpf series, so I need to look at the hash of the flattenend tag.
b48609e
to
4602b01
Compare
While umpf was explicitly developed with multiple developers adding utags and sharing topic branches in mind, it is less than ideal when there are multiple developers working on the same topic branches. And it frequently leads to one of two issues: - The branch is pushed before the utag is accepted into the BSP repository and other developers get an unexpected addition to their umpf - The branch is not pushed after the utag is accepted into the BSP repository and other developers get an unexpected removal from their umpf Every time this happens, it wastes a bit of time to identify what went wrong and thus a solution built into umpf is appropriate: - CI will call umpf --remote=downstream --force push $BSP/series.inc when a PR touching a useries is accepted - Developers can call umpf pull to synchronize their topic branches or to find out when difference they have to the now upstream version Signed-off-by: Ahmad Fatoum <[email protected]>
4602b01
to
14dbf0f
Compare
While umpf was explicitly developed with multiple developers adding utags and sharing topic branches in mind, it is less than ideal when there are multiple developers working on the same topic branches.
And it frequently leads to one of two issues:
The branch is pushed before the utag is accepted into the BSP repository and other developers get an unexpected addition to their umpf
The branch is not pushed after the utag is accepted into the BSP repository and other developers get an unexpected removal from their umpf
Every time this happens, it wastes a bit of time to identify what went wrong and thus a solution built into umpf is appropriate:
CI will call
umpf --remote=downstream --force push $BSP/series.inc
when a PR touching a useries is acceptedDevelopers can call
umpf pull
to synchronize their topic branches or to find out when difference they have to the now upstream version