Re: [PATCH] builtin/push.c: Add `--notes` option

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Sep 20, 2013 at 2:20 PM, Rodolphe Belouin
<rodolphe.belouin@xxxxxxxxx> wrote:
> Make the user able to call `git push --notes` instead of
> `git push refs/notes/*`

I'm sorry for not replying to this earlier. I'm unsure how much of the
earlier discussions around pushing and pulling notes you have
followed, but the short story is that (as demontrated by the script in
[1]) pushing and pulling notes is not as simple as it might initially
seem.

Notes behave more like branches than tags (in that we expect them to
change over time, and we (often) want to collaborate on advancing
them). In order to support distributed collaboration on branches, we
have remote-tracking branches (remotes/$remote/$branch) and associated
behavior in git, which keeps track of the state of other repos, and
supports the integration of remote and local changes. That
infrastructure does not yet exist for notes.

As a result, collaborating on notes quickly becomes painful when you
have to fetch "magic" refspecs and manually orchestrate the
integration of remote and local notes changes (as shown in [1]).

The proposed solution for this is to reorganize the refs/remotes/
hierarchy to allow for remote-tracking notes in addition to
remote-tracking branches (and also other kinds of remote-tracking
refs). However this reorganization is itself a big project and will
not be completed in the short term.

So, how does this apply to your patch?

There is (AFAICS) nothing wrong with the patch itself (except maybe
some added tests would be nice...), but we must consider its
implications and how it guides the use of notes.

First, as you state in the commit message, this allows use of a simple
option instead of a supplying a refspec. And as long as you are the
only person pushing notes into that repo (and you only fast-forward),
this should work well, and lower the barrier for exchanging notes with
a remote repo.

However, as soon as you start collaborating with others, you will need
to integrate their changes, and that's where current Git stops
providing any help: You are left to do the manual integration shown in
[1]. This integration requires you to know about and juggle refspecs
quite intimately, and it could even be argued that the simplicity of
"git push --notes" tricks an unsuspecting user into a future manual
integration regime that is probably outside most git users' comfort
zone...

That said, if there is consensus that "push --notes" is valuable in
the short term (before the larger remote refs reorg is complete), and
that it won't set a user interface precedent that will obviously be
broken by said reorg, then I am not opposed to this patch.


Hope this helps,

...Johan


[1]:
#!/bin/sh

set -e

rm -rf notes_test_area
mkdir notes_test_area
cd notes_test_area

# Prepare server with initial note
git init server
cd server
echo foo>foo
git add foo
git commit -m foo
git notes add -m "Initial note on foo"
echo bar >>foo
git commit -a -m bar
cd ..

# Clone two clients and transfer notes
git clone server clientA
cd clientA
git fetch origin refs/notes/commits:refs/notes/commits
cd ..

git clone server clientB
cd clientB
git fetch origin refs/notes/commits:refs/notes/commits
cd ..

# Add notes in both clients
cd clientA
git notes add -m "clientA's note on bar"
cd ../clientB
git notes add -m "clientB's note on bar"
cd ..

# Push notes from clientA
cd clientA
git push origin 'refs/notes/*' # This works
cd ..

# Push notes from clientB
cd clientB
git push origin 'refs/notes/*' || echo "This command fails!"
# Must merge changes from origin's refs/notes/commits into our own
# refs/notes/commits, but there is no built-in machinery to do so.
# Do it manually instead:
git fetch origin refs/notes/commits:refs/notes/commits_from_origin
git notes merge commits_from_origin || echo "Oops, conflict!"
# Conflict in .git/NOTES_MERGE_WORKTREE/$SHA1. Resolve manually
commit=$(git rev-parse HEAD)
echo "clientA's and clientB's notes on bar" >
".git/NOTES_MERGE_WORKTREE/$commit"
# Commit conflict resolution to finalize notes merge
git notes merge --commit
# Remove temporary placeholder for origin's notes
git update-ref -d refs/notes/commits_from_origin
# Now we can finally try to push again
git push origin 'refs/notes/*' # This works
cd ..

# Behold the end result
cd server
git log --graph -p -c refs/notes/commits
cd ..

echo "done"
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]