Re: [PATCH] tile: support GENERIC_KERNEL_THREAD and GENERIC_KERNEL_EXECVE

Re: [PATCH] tile: support GENERIC_KERNEL_THREAD and GENERIC_KERNEL_EXECVE

[Thomas Gleixner]

On Tue, 23 Oct 2012, Al Viro wrote:
> On Tue, Oct 23, 2012 at 01:30:26PM -0400, Chris Metcalf wrote:
> 
> > I fetched the series from your arch-tile branch and built it, and it works
> > fine.  It looks good from my inspection:
> > 
> > Acked-by: Chris Metcalf <cmetcalf@tilera.com>
> 
> Thanks; Acked-by applied, branch pushed and put into no-rebase mode.
> 
> BTW, something like detached Acked-by objects might be a good idea - i.e.
> commit-like git object with amendment to commit message of a given ancestor.
> The situation when ACKs come only after the commit has been pushed is quite
> common.  Linus, what do you think about usefulness of such thing?  Ability
> to append ACKed-by/Tested-by of an earlier commit to a branch instead of
> git commit --amend + possibly some cherry-picks + force-push, that is.

I agree that this is a common issue. Acked-by/Reviewed-by mails come
in after the fact that the patch has been committed to an immutable
(i.e no-rebase mode) branch or if the change in question already hit
Linus tree.

Still it would be nice to have a recording of that in the git tree
itself.

Something like: "git --attach SHA1 <comment>" would be appreciated!

Thanks,

	tglx

Re: [PATCH] tile: support GENERIC_KERNEL_THREAD and GENERIC_KERNEL_EXECVE

[Jeff King]

On Tue, Oct 23, 2012 at 10:47:28PM +0200, Thomas Gleixner wrote:

> I agree that this is a common issue. Acked-by/Reviewed-by mails come
> in after the fact that the patch has been committed to an immutable
> (i.e no-rebase mode) branch or if the change in question already hit
> Linus tree.
> 
> Still it would be nice to have a recording of that in the git tree
> itself.
> 
> Something like: "git --attach SHA1 <comment>" would be appreciated!

It is spelled:

  git notes add -m <comment> SHA1

The resulting notes are stored in a separate revision-controlled branch
and can be pushed and pulled like regular refs. Note, though, that the
default refspecs do not yet include refs/notes, so you'd have to add
them manually. The workflows around notes are not very mature yet, so if
you start using them, feedback would be appreciated.

-Peff

Re: [PATCH] tile: support GENERIC_KERNEL_THREAD and GENERIC_KERNEL_EXECVE

[Catalin Marinas]

On 23 October 2012 21:51, Jeff King <peff@peff.net> wrote:
> On Tue, Oct 23, 2012 at 10:47:28PM +0200, Thomas Gleixner wrote:
>
>> I agree that this is a common issue. Acked-by/Reviewed-by mails come
>> in after the fact that the patch has been committed to an immutable
>> (i.e no-rebase mode) branch or if the change in question already hit
>> Linus tree.
>>
>> Still it would be nice to have a recording of that in the git tree
>> itself.
>>
>> Something like: "git --attach SHA1 <comment>" would be appreciated!
>
> It is spelled:
>
>   git notes add -m <comment> SHA1
>
> The resulting notes are stored in a separate revision-controlled branch
> and can be pushed and pulled like regular refs. Note, though, that the
> default refspecs do not yet include refs/notes, so you'd have to add
> them manually. The workflows around notes are not very mature yet, so if
> you start using them, feedback would be appreciated.

What would be nice is that notes are pushed/pulled automatically with
standard git push/fetch/pull commands. Usually git walks the DAG
starting with the pulled commit or tag and following the parents. With
notes, the reference is reversed, the note pointing to the commit and
not the other way around. So handling this automatically in Git would
be really useful.

The other feature I'd like is that notes are automatically folded in
the log during git rebase (maybe similar to the squash option). If you
rebase, you lose all the notes (though this depends on the workflow,
it may not be needed with published branches).

-- 
Catalin

Re: [PATCH] tile: support GENERIC_KERNEL_THREAD and GENERIC_KERNEL_EXECVE

[Jeff King]

On Tue, Oct 23, 2012 at 10:09:46PM +0100, Catalin Marinas wrote:

> > It is spelled:
> >
> >   git notes add -m <comment> SHA1
> >
> > The resulting notes are stored in a separate revision-controlled branch
> > and can be pushed and pulled like regular refs. Note, though, that the
> > default refspecs do not yet include refs/notes, so you'd have to add
> > them manually. The workflows around notes are not very mature yet, so if
> > you start using them, feedback would be appreciated.
> 
> What would be nice is that notes are pushed/pulled automatically with
> standard git push/fetch/pull commands. Usually git walks the DAG
> starting with the pulled commit or tag and following the parents. With
> notes, the reference is reversed, the note pointing to the commit and
> not the other way around. So handling this automatically in Git would
> be really useful.

Right, that's what I meant about the refspecs. You can configure git to
push or pull them automatically, but it is not the default. Something
like:

  git config --add remote.origin.fetch '+refs/notes/*:refs/notes/origin/*'

would be a start, but you'd also want to "git notes merge" upstream's
changes into your local notes (you _could_ just fetch straight into
refs/notes/, but if you are making your own notes locally, you have to
resolve it somehow). Exactly how to make this smooth is one of the workflow
considerations; there's been discussion, but most people aren't using
the feature, so we don't have a lot of data.

If you are asking whether we could "auto-follow" notes for commits that
have been fetched like we do for tags, the answer is "not really". The
notes tree is version-controlled as a whole, so you generally fetch the
whole thing or not. And the remote does not advertise note information
the same way we advertise peeled tag references, so a client does not
know which notes are available for fetch. The intended strategy is to
pull in the notes or not (though you can have multiple notes refs with
different names, and fetch just a subset of them).

> The other feature I'd like is that notes are automatically folded in
> the log during git rebase (maybe similar to the squash option). If you
> rebase, you lose all the notes (though this depends on the workflow,
> it may not be needed with published branches).

Git-rebase can automatically copy notes from one commit to another
during a rebase, but you need to set notes.rewriteRef to do so (see "git
help config" for details). The reason for this conservative default is
that some notes may not be appropriate for automatic copying (e.g., a
notes tree containing QA approval should probably be invalidated during
a rebase, whereas one with commentary probably should).

Squashing the notes into the commit message during rebase would be a
useful feature (at least for some type of notes), but that feature does
not currently exist (and as far as I recall, this is the first it has
been proposed).

Again, I think a lot of this comes down to the fact that not many people
are really using notes for their daily workflow, so these itches are not
coming up and getting scratched.

-Peff

Re: [PATCH] tile: support GENERIC_KERNEL_THREAD and GENERIC_KERNEL_EXECVE

[Catalin Marinas]

On Tue, Oct 23, 2012 at 10:22:45PM +0100, Jeff King wrote:
> On Tue, Oct 23, 2012 at 10:09:46PM +0100, Catalin Marinas wrote:
> > > It is spelled:
> > >
> > >   git notes add -m <comment> SHA1
> > >
> > > The resulting notes are stored in a separate revision-controlled branch
> > > and can be pushed and pulled like regular refs. Note, though, that the
> > > default refspecs do not yet include refs/notes, so you'd have to add
> > > them manually. The workflows around notes are not very mature yet, so if
> > > you start using them, feedback would be appreciated.
> > 
> > What would be nice is that notes are pushed/pulled automatically with
> > standard git push/fetch/pull commands. Usually git walks the DAG
> > starting with the pulled commit or tag and following the parents. With
> > notes, the reference is reversed, the note pointing to the commit and
> > not the other way around. So handling this automatically in Git would
> > be really useful.
> 
> Right, that's what I meant about the refspecs. You can configure git to
> push or pull them automatically, but it is not the default. Something
> like:
> 
>   git config --add remote.origin.fetch '+refs/notes/*:refs/notes/origin/*'

Yes, but that's a bit more complicated than a simple pull. Anyway, Linus
seems to not be in favour of annotating commits later for adding acks,
so no need for such feature.

> > The other feature I'd like is that notes are automatically folded in
> > the log during git rebase (maybe similar to the squash option). If you
> > rebase, you lose all the notes (though this depends on the workflow,
> > it may not be needed with published branches).
> 
> Git-rebase can automatically copy notes from one commit to another
> during a rebase, but you need to set notes.rewriteRef to do so (see "git
> help config" for details). The reason for this conservative default is
> that some notes may not be appropriate for automatic copying (e.g., a
> notes tree containing QA approval should probably be invalidated during
> a rebase, whereas one with commentary probably should).

Thanks, I wasn't aware of this.

> Squashing the notes into the commit message during rebase would be a
> useful feature (at least for some type of notes), but that feature does
> not currently exist (and as far as I recall, this is the first it has
> been proposed).

For some workflow - I post patches to the list, people reply with their
acks, I could just add those to notes and later fold them into the
existing commits before pushing the branch upstream. I guess it may be
just a matter of changing git format-patch to include the notes. I can
later reword he commits and drop the "Notes:" line.

-- 
Catalin

Re: [PATCH] tile: support GENERIC_KERNEL_THREAD and GENERIC_KERNEL_EXECVE

[Thomas Gleixner]

On Tue, 23 Oct 2012, Jeff King wrote:

> On Tue, Oct 23, 2012 at 10:47:28PM +0200, Thomas Gleixner wrote:
> 
> > I agree that this is a common issue. Acked-by/Reviewed-by mails come
> > in after the fact that the patch has been committed to an immutable
> > (i.e no-rebase mode) branch or if the change in question already hit
> > Linus tree.
> > 
> > Still it would be nice to have a recording of that in the git tree
> > itself.
> > 
> > Something like: "git --attach SHA1 <comment>" would be appreciated!
> 
> It is spelled:
> 
>   git notes add -m <comment> SHA1

Cool!

> The resulting notes are stored in a separate revision-controlled branch

Which branch(es) is/are that ? What are the semantics of that?

Assume I commit something to branch "foo"

Now I get that late Ack/Reviewed-by and want to associate that to that
commit in branch "foo". Does that go into "notes/foo" ?

If yes, good. (Any other sensible prefix is good as well). If no,
where does it go to?

Later when I send a pull request to my upstream maintainer for branch
"foo" does he get "notes/foo" automagically or do I have to request to
pull him that separately?

Either way is fine for me, though something which lets me "automate"
that would be appreciated. I can work around that easily as my pull
requests are generated via scripts, so I can add the secondary one for
the dependent "notes" branch if necessary. Though it would be nice to
avoid that. Avoiding that, i.e having a straight connection (maybe
configurable) between "foo" and "notes/foo" and the commits which have
not yet hit my upstream maintainer would make my life easier. I.e. I
just have to check "foo" for stuff which is not upstream yet instead
of checking both, but that might just be my laziness.

Thoughts?

	tglx

Re: [PATCH] tile: support GENERIC_KERNEL_THREAD and GENERIC_KERNEL_EXECVE

[Jeff King]

On Tue, Oct 23, 2012 at 11:25:06PM +0200, Thomas Gleixner wrote:

> > The resulting notes are stored in a separate revision-controlled branch
> 
> Which branch(es) is/are that ? What are the semantics of that?

They are stored in refs/notes/commits by default, but you can have
multiple notes refs if you want to store logically distinct sets of
notes.

A notes ref's tree is just a tree whose entries are sha1s, and the file
contents contain the notes themselves (the sha1s are broken down into
subdirectories for performance, but "git notes" handles this behind the
scenes). Technically you could check it out as a branch, edit, and
commit, but "git checkout" is not happy to have a HEAD outside of
refs/heads/, so you are stuck with plumbing like:

  $ git checkout `git rev-parse refs/notes/commits`
  $ edit edit edit
  $ git commit ...
  $ git update-ref refs/notes/commits HEAD

It's probably not good for much beyond exploring how notes are
implemented. See "git help notes" for more discussion.

> Assume I commit something to branch "foo"
> 
> Now I get that late Ack/Reviewed-by and want to associate that to that
> commit in branch "foo". Does that go into "notes/foo" ?

No. It would go into refs/notes/commits, or you could ask it to go to
refs/notes/acks if you wanted to keep them separate from your default
notes. It is indexed by commit object, not by branch (so if that branch
later goes away, the notes are always still attached to the commit
objects, assuming they got merged in).

> Later when I send a pull request to my upstream maintainer for branch
> "foo" does he get "notes/foo" automagically or do I have to request to
> pull him that separately?

No, he would have to pull your notes separately. Most of the discussion
around sharing has been configuring the default refspec configuration to
fetch notes.  But in the kernel you guys use a lot of one-off pulls
without configured remotes. I'm not sure what the right workflow would
be. It might simply be to ask git to always pull particular notes
commits at the same time (so you might push your notes to
refs/notes/for-linus, and then git would automatically grab the notes
when somebody pulls refs/heads/for-linus).

> Either way is fine for me, though something which lets me "automate"
> that would be appreciated. I can work around that easily as my pull
> requests are generated via scripts, so I can add the secondary one for
> the dependent "notes" branch if necessary. Though it would be nice to
> avoid that. Avoiding that, i.e having a straight connection (maybe
> configurable) between "foo" and "notes/foo" and the commits which have
> not yet hit my upstream maintainer would make my life easier. I.e. I
> just have to check "foo" for stuff which is not upstream yet instead
> of checking both, but that might just be my laziness.
> 
> Thoughts?

That all makes sense. Putting extra work on the puller is not a good
long-term solution. So while sending them an extra "also pull these
notes" line, even if it ends up being a cut-and-pastable single-liner,
is not great (even if it is the most flexible thing). Using a convention
based on name-equivalence seems like a sensible compromise.

-Peff

RE: [PATCH] tile: support GENERIC_KERNEL_THREAD and GENERIC_KERNEL_EXECVE

[Marc Gauthier]

Jeff King wrote:
> On Tue, Oct 23, 2012 at 11:25:06PM +0200, Thomas Gleixner wrote:
> > > The resulting notes are stored in a separate
> > > revision-controlled branch
> >
> > Which branch(es) is/are that ? What are the semantics of that?
[...]


Nice feature.

Can a later commit be eventually be made to reference some set
of notes added so far, so they become part of the whole history
signed by the HEAD SHA1?  hence pulled/pushed automatically as
well.  Otherwise do you not end up with a forever growing separate
tree of notes that loses some of the properties of being behind
the head SHA1 (and perhaps less scalable in manageability)?
Also that way notes are separate only temporarily.

As for automating the inclusion of notes in the flow, can that
be conditional on some pattern in the note, so that e.g. the
Acked-by's get included and folded in automatically, whereas
others do not, according to settings?

-Marc

Re: [PATCH] tile: support GENERIC_KERNEL_THREAD and GENERIC_KERNEL_EXECVE

[Jeff King]

On Tue, Oct 23, 2012 at 03:06:59PM -0700, Marc Gauthier wrote:

> Can a later commit be eventually be made to reference some set
> of notes added so far, so they become part of the whole history
> signed by the HEAD SHA1?  hence pulled/pushed automatically as
> well.  Otherwise do you not end up with a forever growing separate
> tree of notes that loses some of the properties of being behind
> the head SHA1 (and perhaps less scalable in manageability)?
> Also that way notes are separate only temporarily.

Interesting idea. It would be tough to do with existing objects. There
are really only two ways for a commit to reference objects:

  1. Via a parent header. But we would not want to include the notes
     tree as a separate parent. The semantics are all wrong, and would
     make your commit look like a nonsense merge.

  2. As an entry in a tree. But we do not enforce connectivity of
     commits referenced in trees, because that is the way that
     submodules are implemented.

So I think we would have to add a new header that says "also, here are
some notes for my history". That has two problems:

  1. It's not backwards compatible. People with the new version of git
     will expect to have objects referenced by the new header, but older
     servers may not provide those objects (and vice versa). We can add
     a protocol extension to communicate this, but fundamentally you are
     going to lose the object connection any time it passes through a
     repo running an older git.

  2. It's impure from the perspective of git's data model. Adding in the
     notes reference is not really a property of the commit. It's more
     like saying "Oh, these other things happened to _past_ commits, and
     I'm just now mentioning them". So you pick an arbitrary commit to
     attach it to. What are the semantics with relation to that commit's
     position in the history graph? If I have a commit that is identical
     but without the notes reference, it will have a different sha1. But
     is it the same commit?

So it's a bit ugly. I think I'd rather build out the transfer
infrastructure to pass the notes references around more gracefully
without trying to shoehorn them into the commit graph.

> As for automating the inclusion of notes in the flow, can that
> be conditional on some pattern in the note, so that e.g. the
> Acked-by's get included and folded in automatically, whereas
> others do not, according to settings?

Yeah. You can store arbitrary data in notes (e.g., one of the existing
uses of notes is to record metadata on the patch emails that led to a
commit). Right now you typically separate it out by data type into
separate refs, and then you ask git log to show you particular ones (so
we see refs/notes/commits with "--notes", but you can do "--notes=foo"
to see refs/notes/foo, or even show multiple refs).

For the fold-on-rebase idea, I'd think you would want something similar,
like setting rebase.foldNotes to "foo" to say "refs/notes/foo contains
pseudo-headers that should be folded in like a signed-off-by".

-Peff

Re: [PATCH] tile: support GENERIC_KERNEL_THREAD and GENERIC_KERNEL_EXECVE

[Johannes Sixt]

Am 10/24/2012 0:23, schrieb Jeff King:
> For the fold-on-rebase idea, I'd think you would want something similar,
> like setting rebase.foldNotes to "foo" to say "refs/notes/foo contains
> pseudo-headers that should be folded in like a signed-off-by".

If you are rebasing anyway, you can already use interactive rebase's
--autosquash option:

# a late ACK came in:
git commit --allow-empty -m'squash! tile: support GENERIC_

Acked-by: A U Thor <author@example.com>'

git rebase -i --keep-empty --autosquash $forkpoint

Requires git 1.7.11 for --keep-empty and requires to edit out the
'squash!...' headers when the editor appears during the rebase.

-- Hannes

Re: [PATCH] tile: support GENERIC_KERNEL_THREAD and GENERIC_KERNEL_EXECVE

[Linus Torvalds]

On Wed, Oct 24, 2012 at 12:25 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
>>
>> It is spelled:
>>
>>   git notes add -m <comment> SHA1
>
> Cool!

Don't use them for anything global.

Use them for local codeflow, but don't expect them to be distributed.
It's a separate "flow", and while it *can* be distributed, it's not
going to be for the kernel, for example. So no, don't start using this
to ack things, because the acks *will* get lost.

             Linus

Re: [PATCH] tile: support GENERIC_KERNEL_THREAD and GENERIC_KERNEL_EXECVE

[Al Viro]

On Wed, Oct 24, 2012 at 04:02:49AM +0300, Linus Torvalds wrote:
> On Wed, Oct 24, 2012 at 12:25 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> >>
> >> It is spelled:
> >>
> >>   git notes add -m <comment> SHA1
> >
> > Cool!
> 
> Don't use them for anything global.
> 
> Use them for local codeflow, but don't expect them to be distributed.
> It's a separate "flow", and while it *can* be distributed, it's not
> going to be for the kernel, for example. So no, don't start using this
> to ack things, because the acks *will* get lost.

How about git commit --allow-empty, with
"belated ACK for <commit>

Acked-by: <...>
" as commit message?  I mean, that ought to work and propagate sanely,
but I'm really not sure if that's something in a good taste and should
be allowed as a common practice...

Re: [PATCH] tile: support GENERIC_KERNEL_THREAD and GENERIC_KERNEL_EXECVE

[Linus Torvalds]

On Wed, Oct 24, 2012 at 4:56 AM, Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> How about git commit --allow-empty, with
> "belated ACK for <commit>

Don't bother. It's not that important, and it's just distracting.

It's not like this is vital information. If you pushed it out without
the ack, it's out without the ack. Big deal.

          Linus

Re: [PATCH] tile: support GENERIC_KERNEL_THREAD and GENERIC_KERNEL_EXECVE

[Ingo Molnar]

* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Wed, Oct 24, 2012 at 12:25 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> >>
> >> It is spelled:
> >>
> >>   git notes add -m <comment> SHA1
> >
> > Cool!
> 
> Don't use them for anything global.
> 
> Use them for local codeflow, but don't expect them to be 
> distributed. It's a separate "flow", and while it *can* be 
> distributed, it's not going to be for the kernel, for example. 
> So no, don't start using this to ack things, because the acks 
> *will* get lost.

I'd also add a small meta argument: that it would be actively 
wrong to *allow* 'belated' acks to be added. In practice acks 
are most useful *before* a commit gets created and they often 
have a mostly buerocratic role afterwards.

So we should encourage timely acks (which actually help 
development), and accept ack-less patches as long as they are 
correct and create no problems. More utility, less buerocracy. 
Incorrect, ack-less patches causing problems will get all the 
flames they deserve.

Thanks,

	Ingo