Merge lp://staging/~robru/bileto/git-support into lp://staging/bileto

Known problems:

1. debcommit is broken:

+ git add --all
+ debcommit --all
debcommit: unable to determine commit message using git (do you mean "debcommit -a" or did you forget to run "git add"?)

¯\_(ツ)_/¯

2. I did a really terrible job of taking advantage of git repos and spend a lot of time schlepping git repos around as if they're just branches.

3. Git seems to be completely lacking the 'bzr missing' feature, which is able to identify commits unique to remote branches. In git I had to fetch the branch and then 'git log' it, and this works, but boy if anybody knows a way to do this remotely without downloading the branch first I'd sure love to hear about that.

In particular, the local git-cache/ dir contains separate repos for the target trunk branch and the working branch (the staged branch prior to merging to trunk). Would like to merge those, would greatly reduce disk space usage.

Ok I believe #2 is now resolved (target master & working branches are now cached in the same repo, halving disk space usage, or more, if there's multiple tickets). Needs more testing of course, and #1 is still a problem.

#1 resolved. Need reviews!

Not familiar with the bileto code. Most of the git interactions are sane.

On Tue, Aug 30, 2016 at 5:40 AM, Dimitri John Ledkov
<email address hidden> wrote:
> Not familiar with the bileto code. Most of the git interactions are sane.

Thanks for the review, yes I was specifically looking for feeback on
my use of git.

>> +git_fetch_wrapper() {
>> + loudly git -C "$1" fetch --update-head-ok --force "$2" "$3"
>
> nice, +1

Why do you think this is nice? I was a bit terrified of this when I
wrote it; without --update-head-ok I was getting fatal errors where it
refused to update HEAD, then I found this option but the manpage for
it hilariously says it's not for end users, only for git developers
implementing new git features, so it's unclear to me how "safe" this
is to use. I've tested it and it appears to work but I'm still unsure
if this is the best approach.

>> +git_do_tag_release() {
>> + do_tag_release
>> + loudly git commit --all --message "Releasing $VERSION" || die "Failed to commit release."
>> + loudly git tag --force --local-user "$(gpg_key)" "$VERSION" --message "Releasing $VERSION" || die "Failed to tag release."
>
> Do you have a gpg key available? in that case a signed tag is better, e.g. -s / --sign option. Unless this is specified already elsewhere via a config file or an alias.

Yes, --sign option fails to determine the correct signing key, so
--local-user "$(gpg_key)" is the equivalent way to create a signed tag
while specifying which key.

>> +git_do_expose() {
>> + loudly git push "$BRANCH" "$GIT_REF:$GIT_REF" --prune --force --tags || die "Failed to push source tree to $BRANCH:$GIT_REF."
>
> Subtle differences may emerge thus better to use --follow-tags instead of --tags.

What kind of subtle differences do you have in mind? The branch being
pushed will have precisely one extra tag vs. what was originally
fetched. I've tested this code and it successfully pushes the tag I
want while also --prune'ing old orphaned tags from previous pushes
that are no longer needed, which is exactly what I want.

>> +git_do_find_new_commits() {
>> + me="$(git config user.email)"
>> + format="--format=format:revision-id: %ce %ci (%cr) %H"
>> + target_has="$WORKPARENT/$$-git.target.has"
>> + missing="$WORKPARENT/$$-git.missing"
>> + cd "$CACHE"
>> + git_fetch_wrapper "$CACHE" "$TARGET" "$GIT_TARGET_REF:$GIT_WORKING_REF-merge-target" || die "Failed to fetch $TARGET."
>> + git_fetch_wrapper "$CACHE" "$BRANCH" "$GIT_SOURCE_REF:$GIT_WORKING_REF-merge-source" || die "Failed to fetch $BRANCH."
>> + # Step 1: Identify commits that the target trunk has that the input branch doesn't (these will be ignored)
>> + stdout git log "$GIT_WORKING_REF-merge-source..$GIT_WORKING_REF-merge-target" "$format" | tee "$target_has.tmp" 1>&2
>> + ignore_translators <"$target_has.tmp" >"$target_has"
>> + # Step 2: Identify new commits that require building.
>> + stdout git log "$GIT_WORKING_REF..$GIT_WORKING_REF-merge-source" "$format" | tee "$missing" 1>&2
>> + ignore_translators <"$missing"
>> + # Step 3: Identify commits that have been deleted from the input branch
>> + # (commits missing from input branch that aren't already on target trunk, or committed by me)
>> + stdou...

> On Tue, Aug 30, 2016 at 5:40 AM, Dimitri John Ledkov
> <email address hidden> wrote:
> > Not familiar with the bileto code. Most of the git interactions are sane.
>
> Thanks for the review, yes I was specifically looking for feeback on
> my use of git.
>
>
> >> +git_fetch_wrapper() {
> >> + loudly git -C "$1" fetch --update-head-ok --force "$2" "$3"
> >
> > nice, +1
>
> Why do you think this is nice? I was a bit terrified of this when I
> wrote it; without --update-head-ok I was getting fatal errors where it
> refused to update HEAD, then I found this option but the manpage for
> it hilariously says it's not for end users, only for git developers
> implementing new git features, so it's unclear to me how "safe" this
> is to use. I've tested it and it appears to work but I'm still unsure
> if this is the best approach.
>

Hm... what are examples of things you fetch though? Ideally one should treat fetch as:

 * get me something remote and give it local committish... name
 * and refer to it as FETCH_HEAD until next time fetch is done".

E.g.

$ git fetch lp:somerepo fix-gcc-warnings
$ git merge --no-ff --no-commit FETCH_HEAD
$ debcommit -a
... or some such.

One should not checkout FETCH_HEAD instead
* update your working directory to it: reset --hard FETCH_HEAD
* merge that into your HEAD with: merge FETCH_HEAD
* start a new branch with that as a start: branch new-branch FETCH_HEAD
* start a new branch, and update working directory to it: checkout -b new-branch FETCH_HEAD

>
> >> +git_do_tag_release() {
> >> + do_tag_release
> >> + loudly git commit --all --message "Releasing $VERSION" || die "Failed
> to commit release."
> >> + loudly git tag --force --local-user "$(gpg_key)" "$VERSION" --message
> "Releasing $VERSION" || die "Failed to tag release."
> >
> > Do you have a gpg key available? in that case a signed tag is better, e.g.
> -s / --sign option. Unless this is specified already elsewhere via a config
> file or an alias.
>
> Yes, --sign option fails to determine the correct signing key, so
> --local-user "$(gpg_key)" is the equivalent way to create a signed tag
> while specifying which key.
>

Ah, something new for me to learn =)

>
> >> +git_do_expose() {
> >> + loudly git push "$BRANCH" "$GIT_REF:$GIT_REF" --prune --force --tags
> || die "Failed to push source tree to $BRANCH:$GIT_REF."
> >
> > Subtle differences may emerge thus better to use --follow-tags instead of
> --tags.
>
> What kind of subtle differences do you have in mind? The branch being
> pushed will have precisely one extra tag vs. what was originally
> fetched. I've tested this code and it successfully pushes the tag I
> want while also --prune'ing old orphaned tags from previous pushes
> that are no longer needed, which is exactly what I want.
>

Excatly that. By default people do not want to prune old orphaned tags. If that is the intention, that's the right approach then. Usually people actually want --follow-tags but don't know about it. All is good then.

>
> >> +git_do_find_new_commits() {
> >> + me="$(git config user.email)"
> >> + format="--format=format:revision-id: %ce %ci (%cr) %H"
> >> + target_has="$WORKPARENT/$$-gi...

On Tue, Aug 30, 2016 at 9:31 AM, Dimitri John Ledkov
<email address hidden> wrote:
>> >> +git_fetch_wrapper() {
>> >> + loudly git -C "$1" fetch --update-head-ok --force "$2" "$3"
>> >
>> > nice, +1
>>
>> Why do you think this is nice? I was a bit terrified of this when I
>> wrote it; without --update-head-ok I was getting fatal errors where it
>> refused to update HEAD, then I found this option but the manpage for
>> it hilariously says it's not for end users, only for git developers
>> implementing new git features, so it's unclear to me how "safe" this
>> is to use. I've tested it and it appears to work but I'm still unsure
>> if this is the best approach.
>>
>
> Hm... what are examples of things you fetch though? Ideally one should treat fetch as:
>
> * get me something remote and give it local committish... name
> * and refer to it as FETCH_HEAD until next time fetch is done".
>
> E.g.
>
> $ git fetch lp:somerepo fix-gcc-warnings
> $ git merge --no-ff --no-commit FETCH_HEAD
> $ debcommit -a
> ... or some such.
>
> One should not checkout FETCH_HEAD instead
> * update your working directory to it: reset --hard FETCH_HEAD
> * merge that into your HEAD with: merge FETCH_HEAD
> * start a new branch with that as a start: branch new-branch FETCH_HEAD
> * start a new branch, and update working directory to it: checkout -b new-branch FETCH_HEAD

We use fetch for **everything**, we never clone as clone doesn't allow
enough control to decide which branch to get and where to store it
locally. So:

* first we fetch target master branch and name it some unique local
name (some name unique to the ticket it's part of so that it doesn't
conflict with other tickets working on the same target master)
* then we fetch all the source branches and merge those in
* then this is pushed to lp and the local copy deleted
* later we fetch the working branch from lp

>> >> +git_do_expose() {
>> >> + loudly git push "$BRANCH" "$GIT_REF:$GIT_REF" --prune --force --tags
>> || die "Failed to push source tree to $BRANCH:$GIT_REF."
>> >
>> > Subtle differences may emerge thus better to use --follow-tags instead of
>> --tags.
>>
>> What kind of subtle differences do you have in mind? The branch being
>> pushed will have precisely one extra tag vs. what was originally
>> fetched. I've tested this code and it successfully pushes the tag I
>> want while also --prune'ing old orphaned tags from previous pushes
>> that are no longer needed, which is exactly what I want.
>>
>
> Excatly that. By default people do not want to prune old orphaned tags. If that is the intention, that's the right approach then. Usually people actually want --follow-tags but don't know about it. All is good then.

Yes, what happens is, every time a ticket is built, a new release
commit with a new tag is pushed, deleting the old release commit and
release tag from the previous build, until finally a build is approved
for actual release. This is actually a serious problem in bzr right
now, we delete the previous release commit but not the previous
release tag so bzr branches managed by bileto have mountains of
orphaned tags, some users complain that tags are meaningless becuse
they're drown out ...