paint the bikeshed: new refs/for/ syntax

150 views
Skip to first unread message

Shawn Pearce

unread,
Feb 24, 2013, 10:55:36 PM2/24/13
to repo-discuss
https://gerrit-review.googlesource.com/42652

tl;dr: Git won't modify the protocol to let us pass more data. Define
a new refs/@ prefix that uses options embedded in the reference
specification:

refs/@master/r=alice
refs/@master/r=alice/topic=bug42
refs/@master/r=alice/cc=bob,charlie

My goal is to let `repo upload` pass its --re/--cc flags to Gerrit
over HTTP where we can't rely on the SSH parser currently used. Its a
bonus if we can make this definition be "extensible" in the same way
that command line options are least named and unique.


I considered several alternatives:

refs/for/master/bug42%r=alice,cc=bob,charlie
# splitting on , confuses something like cc

refs/for/master/bug42%{r=alice}{cc=bob,charlie}
# fixes the comma issue, but is maybe too ugly

ref/for/master/bug42/r=alice,cc=bob,charlie
# maybe too ambiguous to parse the options?

refs/for/master/bug42/r=alice/cc=bob,charlie
# also a bit ugly to parse


Something I haven't considered but we may need to is allowing plugins
to add their own options here. Specifically a commit validator plugin
might want to let the user say "ignore the validator" e.g. in a case
where the user is importing a commit done by someone else and the
validator shouldn't reject its style.

Shawn Pearce

unread,
Feb 24, 2013, 11:09:17 PM2/24/13
to repo-discuss
On Sun, Feb 24, 2013 at 7:55 PM, Shawn Pearce <s...@google.com> wrote:
> https://gerrit-review.googlesource.com/42652
>
> tl;dr: Git won't modify the protocol to let us pass more data. Define
> a new refs/@ prefix that uses options embedded in the reference
> specification:
>
> refs/@master/r=alice

I should have mentioned the design constraints:

- All characters must be valid in a Git reference name:

The git push client refuses to send a bad name to the server. We are
therefore restricted to what Git thinks is a reasonable branch name.
You can test locally by doing experiments with check-ref-format, e.g.:

git check-ref-format --branch master..{r=alice}

- Avoid common shell metacharacters:

I don't want to need quotes around the reference spec, or to use \ to
avoid my shell from reading something. So $ is out.

I think the subset of tokens we are left with is:

@ , / {}

Git accepts things like !, $, <> and ; but these are special to most
shells and would need quoting. Git already rejects things like ~ : and
.. so we can't use those.

Dave Borowitz

unread,
Feb 25, 2013, 12:12:45 AM2/25/13
to Shawn Pearce, repo-discuss
Any possibility for escape sequences for characters otherwise invalid in ref names? % is valid, right, so could we do URL escaping?



--
--
To unsubscribe, email repo-discuss...@googlegroups.com
More info at http://groups.google.com/group/repo-discuss?hl=en

---
You received this message because you are subscribed to the Google Groups "Repo and Gerrit Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to repo-discuss...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.



Shawn Pearce

unread,
Feb 25, 2013, 12:55:44 AM2/25/13
to Dave Borowitz, repo-discuss
On Sun, Feb 24, 2013 at 9:12 PM, Dave Borowitz <dbor...@google.com> wrote:
> Any possibility for escape sequences for characters otherwise invalid in ref
> names? % is valid, right, so could we do URL escaping?

Yes % is valid so we could do %20 to insert a space for example. I
hadn't thought that far ahead.

Dave Borowitz

unread,
Feb 25, 2013, 1:15:15 AM2/25/13
to Shawn Pearce, repo-discuss
Yeah, arbitrary characters aren't necessarily needed for the builtin Gerrit options, but plugin authors might be more interested.

I was thinking it might also be worth packaging a shell script with Gerrit (gerrit-push?) that does the option->refname conversion for you, especially if escaping is involved.

Shawn Pearce

unread,
Feb 25, 2013, 1:27:13 AM2/25/13
to Dave Borowitz, repo-discuss
On Sun, Feb 24, 2013 at 10:15 PM, Dave Borowitz <dbor...@google.com> wrote:
> Yeah, arbitrary characters aren't necessarily needed for the builtin Gerrit
> options, but plugin authors might be more interested.
>
> I was thinking it might also be worth packaging a shell script with Gerrit
> (gerrit-push?) that does the option->refname conversion for you, especially
> if escaping is involved.

Well, this was basically Junio's argument for why Git won't extend the
network protocol. His suggestion was we pack the data into an
annotated tag, e.g.:

git push $((echo "object $(git rev-parse HEAD)";
echo "type commit";
echo "tag tmp";
echo "tagger $(git var GIT_COMMITTER_IDENT)";
echo;
echo "-r=alice";
echo "--cc=bob") | git mktag):refs/for/master

which basically demands a script. I've been trying to avoid a script.

Edwin Kempin

unread,
Feb 25, 2013, 1:28:01 AM2/25/13
to Shawn Pearce, repo-discuss


2013/2/25 Shawn Pearce <s...@google.com>

https://gerrit-review.googlesource.com/42652

tl;dr: Git won't modify the protocol to let us pass more data. Define
a new refs/@ prefix that uses options embedded in the reference
specification:

  refs/@master/r=alice
  refs/@master/r=alice/topic=bug42
  refs/@master/r=alice/cc=bob,charlie
How would this ref look like if I want to push something for review to the refs/meta/config branch?

What about placing the @ in the very beginning to say it's for review:
  @master/r=alice/topic=bug42/cc=bob,charlie
or in the long version:
  @refs/heads/master/r=alice/topic=bug42/cc=bob,charlie
?
 

My goal is to let `repo upload` pass its --re/--cc flags to Gerrit
over HTTP where we can't rely on the SSH parser currently used. Its a
bonus if we can make this definition be "extensible" in the same way
that command line options are least named and unique.


I considered several alternatives:

  refs/for/master/bug42%r=alice,cc=bob,charlie
  # splitting on , confuses something like cc

  refs/for/master/bug42%{r=alice}{cc=bob,charlie}
  # fixes the comma issue, but is maybe too ugly

  ref/for/master/bug42/r=alice,cc=bob,charlie
  # maybe too ambiguous to parse the options?

  refs/for/master/bug42/r=alice/cc=bob,charlie
  # also a bit ugly to parse


Something I haven't considered but we may need to is allowing plugins
to add their own options here. Specifically a commit validator plugin
might want to let the user say "ignore the validator" e.g. in a case
where the user is importing a commit done by someone else and the
validator shouldn't reject its style.

Shawn Pearce

unread,
Feb 25, 2013, 1:40:30 AM2/25/13
to Edwin Kempin, repo-discuss
On Sun, Feb 24, 2013 at 10:28 PM, Edwin Kempin <edwin....@gmail.com> wrote:
> 2013/2/25 Shawn Pearce <s...@google.com>
>>
>> https://gerrit-review.googlesource.com/42652
>>
>> tl;dr: Git won't modify the protocol to let us pass more data. Define
>> a new refs/@ prefix that uses options embedded in the reference
>> specification:
>>
>> refs/@master/r=alice
>> refs/@master/r=alice/topic=bug42
>> refs/@master/r=alice/cc=bob,charlie
>
> How would this ref look like if I want to push something for review to the
> refs/meta/config branch?

refs/@refs/meta/config

> What about placing the @ in the very beginning to say it's for review:
> @master/r=alice/topic=bug42/cc=bob,charlie
> or in the long version:
> @refs/heads/master/r=alice/topic=bug42/cc=bob,charlie
> ?

The Git client will not accept that. So no.

Edwin Kempin

unread,
Feb 25, 2013, 2:00:24 AM2/25/13
to Shawn Pearce, repo-discuss


2013/2/25 Shawn Pearce <s...@google.com>
Ok, I think what I wanted to propose is then
  refs/heads/@master/r=alice
because this allows you to omit the 'refs/heads/' prefix on the command line and you could just refer to it as
  @master/r=alice

This looks somehow simpler from the user's perspective.

/c/test/testMe ((dd04140...))
$ git branch @master/r=alice/cc=john...@example.com

/c/test/testMe ((dd04140...))
$ git branch
* (no branch)
  @master/r=alice/cc=john...@example.com
  master

But I can see why you don't want to have this in the 'refs/heads/' namespace...



Shawn Pearce

unread,
Feb 25, 2013, 2:21:12 AM2/25/13
to Edwin Kempin, repo-discuss
Specifically what is happening here is the client requires the RHS to
begin with "refs/" before it sends the string to the server. When the
RHS does not begin with "refs/" then the client enters its matching
routine where it tries to find a reference listed by the server that
matches the RHS and expands the RHS to what was given by the server.

Making the client implement its matching rules means we have to
advertise every branch also as its refs/for/ or refs/@ version. This
not only doubles the size of the advertisement, it also confuses the
client when the remote branch already exists and is ahead of the
change you are trying to upload. Gerrit allows you to upload something
that is slightly out of date because the server will do the merge for
you if its necessary. The only way that works is to have the server
*not* send the advertisement for the branch space we use. Which means
we need to begin with refs/ on the RHS anytime we talk to Gerrit and
want magic support.

Shawn Pearce

unread,
Feb 25, 2013, 2:22:16 AM2/25/13
to Edwin Kempin, repo-discuss
On Sun, Feb 24, 2013 at 11:00 PM, Edwin Kempin <edwin....@gmail.com> wrote:
>> > What about placing the @ in the very beginning to say it's for review:
>> > @master/r=alice/topic=bug42/cc=bob,charlie
>> > or in the long version:
>> > @refs/heads/master/r=alice/topic=bug42/cc=bob,charlie
>> > ?
>>
>> The Git client will not accept that. So no.
>
> Ok, I think what I wanted to propose is then
> refs/heads/@master/r=alice
> because this allows you to omit the 'refs/heads/' prefix on the command line
> and you could just refer to it as
> @master/r=alice
>
> This looks somehow simpler from the user's perspective.
>
> /c/test/testMe ((dd04140...))
> $ git branch @master/r=alice/cc=john...@example.com

Sure but you tested with git branch. Retry your test with git push. :-(

Edwin Kempin

unread,
Feb 25, 2013, 2:31:00 AM2/25/13
to Shawn Pearce, repo-discuss


2013/2/25 Shawn Pearce <s...@google.com>
OK, I see :-(
  error: unable to push to unqualified destination: @master/r=alice/cc=john.doe@exa
  The destination refspec neither matches an existing ref on the remote nor
  begins with refs/, and we are unable to guess a prefix based on the source ref.
 

Thomas Swindells (tswindel)

unread,
Feb 25, 2013, 4:49:14 AM2/25/13
to Edwin Kempin, Shawn Pearce, repo-discuss

I’m probably missing something obvious but can you explain why this is too hard to pass?

  refs/for/master/bug42/r=alice/cc=bob,charlie

  # also a bit ugly to parse

 

Isn’t it just a case of finding the first path segment with an equals sign in, anything before that is part of the ref name (and matched as it currently is), all segments after this must have an equals sign in and are treated as key-value pairs?

 

This has the advantage that the push syntax stays fundamentally the same with just some additional optional extensions at the end.

I assume that ‘?’ is not a permitted character so we can’t just treat it as a URI:

refs/for/master/bug42?r=alice&cc=bob,charlie

 

Thomas

--

Alex Blewitt

unread,
Feb 25, 2013, 4:56:47 AM2/25/13
to Shawn Pearce, repo-discuss
On 25 Feb 2013, at 03:55, Shawn Pearce wrote:

> https://gerrit-review.googlesource.com/42652
>
> tl;dr: Git won't modify the protocol to let us pass more data. Define
> a new refs/@ prefix that uses options embedded in the reference
> specification:
>
> refs/@master/r=alice
> refs/@master/r=alice/topic=bug42
> refs/@master/r=alice/cc=bob,charlie

Instead of refs/@master, did you consider refs/@/master? This might make parsing or splitting slightly easier, especially when the branch you're pushing to isn't just a single name (like 'master') but a nested reference (e.g. bugs/123/).

It might also be worth using a stop-character after the branch name to disambiguate where the reference ends and the value set passes, e.g.:

refs/@/bugs/123/@/r=alice/topic=bug123/cc=bob,charlie

Is the requirement that the refs be readable by a human? If not, you could use base64 encoded JSON:

(master) $ base64
{"bug":123,"cc":["bob","charlie"],"r":"alice"}
eyJidWciOjEyMywiY2MiOlsiYm9iIiwiY2hhcmxpZSJdLCJyIjoiYWxpY2UifQo=

(master) $ git push gerrit master:refs/@/master/@/eyJidWciOjEyMywiY2MiOlsiYm9iIiwiY2hhcmxpZSJdLCJyIjoiYWxpY2UifQo=
Total 0 (delta 0), reused 0 (delta 0)
To gerrit
* [new branch] master -> refs/@/master/@/eyJidWciOjEyMywiY2MiOlsiYm9iIiwiY2hhcmxpZSJdLCJyIjoiYWxpY2UifQo=

There might be some mangle/de-mangling required for + and = in the HTTP requests, but the = only appears at the end as a padding character and thus can be inferred, and the + may be swapped with a space by some clients (but a % or space can't appear in the output of a base64 encoded string anyway).

If + isn't allowed, then one could use @ instead of + for encoding 62.

Alex

Shawn Pearce

unread,
Feb 25, 2013, 11:52:35 AM2/25/13
to Thomas Swindells (tswindel), Edwin Kempin, repo-discuss
On Mon, Feb 25, 2013 at 1:49 AM, Thomas Swindells (tswindel)
<tswi...@cisco.com> wrote:
> I’m probably missing something obvious but can you explain why this is too
> hard to pass?
>
> refs/for/master/bug42/r=alice/cc=bob,charlie
>
> # also a bit ugly to parse

Actually, this is what my latest patch set does. I tried it after I
wrote this email. :-)

> Isn’t it just a case of finding the first path segment with an equals sign
> in, anything before that is part of the ref name (and matched as it
> currently is), all segments after this must have an equals sign in and are
> treated as key-value pairs?

Eh, sort of. I implemented it a little bit differently but yes.

> This has the advantage that the push syntax stays fundamentally the same
> with just some additional optional extensions at the end.

Yes.

> I assume that ‘?’ is not a permitted character so we can’t just treat it as
> a URI:

? is permitted but I hate the way this reads:

> refs/for/master/bug42?r=alice&cc=bob,charlie

Ick. Especially since & needs to be quoted from the shell. Which I
realized {cc=a,b} would also need to be quoted so good thing I didn't
try to implement that.

Shawn Pearce

unread,
Feb 25, 2013, 11:58:09 AM2/25/13
to Alex Blewitt, repo-discuss
On Mon, Feb 25, 2013 at 1:56 AM, Alex Blewitt <alex.b...@gmail.com> wrote:
> On 25 Feb 2013, at 03:55, Shawn Pearce wrote:
>
>> https://gerrit-review.googlesource.com/42652
>>
>> tl;dr: Git won't modify the protocol to let us pass more data. Define
>> a new refs/@ prefix that uses options embedded in the reference
>> specification:
>>
>> refs/@master/r=alice
>> refs/@master/r=alice/topic=bug42
>> refs/@master/r=alice/cc=bob,charlie
>
> Instead of refs/@master, did you consider refs/@/master? This might make parsing or splitting slightly easier, especially when the branch you're pushing to isn't just a single name (like 'master') but a nested reference (e.g. bugs/123/).

Yes but it doesn't make parsing any easier. It just makes the human
type an extra character. Basically I am betting that nobody is crazy
enough to create "refs/@mystuff/dont.look.here" in their Git
repository, just like Gerrit already assumes you aren't going to
create refs/for/master or refs/changes/01/1000/1.

> It might also be worth using a stop-character after the branch name to disambiguate where the reference ends and the value set passes, e.g.:
>
> refs/@/bugs/123/@/r=alice/topic=bug123/cc=bob,charlie

Doesn't really help.

> Is the requirement that the refs be readable by a human? If not, you could use base64 encoded JSON:

No, but I want it easy for a human to type on the command line without
needing extra utilities. I really wanted to be able to say:

$ git push gerrit -r r=alice -r cc=bob,charlie HEAD:refs/for/master

However the Git maintainer rejected the idea of adding a -r flag to
pass data to the server side of the connection. So now I am trying to
approximate this.

> (master) $ base64
> {"bug":123,"cc":["bob","charlie"],"r":"alice"}
> eyJidWciOjEyMywiY2MiOlsiYm9iIiwiY2hhcmxpZSJdLCJyIjoiYWxpY2UifQo=
>
> (master) $ git push gerrit master:refs/@/master/@/eyJidWciOjEyMywiY2MiOlsiYm9iIiwiY2hhcmxpZSJdLCJyIjoiYWxpY2UifQo=
> Total 0 (delta 0), reused 0 (delta 0)
> To gerrit
> * [new branch] master -> refs/@/master/@/eyJidWciOjEyMywiY2MiOlsiYm9iIiwiY2hhcmxpZSJdLCJyIjoiYWxpY2UifQo=

Someone did propose this internally at Google to me, so its not a new
idea. I hate it because you need an extra command to perform the
base64 encoding. You can wedge this all into a single command line of
course:

git push gerrit HEAD:refs/@master/$(echo
'{"bug":123,"cc":["bob","charlie"],"r":"alice"}' | base64)

but this is fugly to type out by hand. If I need to go to $() or ``
then I have already lost.

Junio suggested using an annotated tag, see one of my earlier replies
for an example of that.

Jean-Baptiste Queru

unread,
Feb 25, 2013, 1:04:03 PM2/25/13
to repo-discuss
Please avoid base64 encoding if possible (or anything else that
requires an extra script).

JBQ
> --
> --
> To unsubscribe, email repo-discuss...@googlegroups.com
> More info at http://groups.google.com/group/repo-discuss?hl=en
>
> ---
> You received this message because you are subscribed to the Google Groups "Repo and Gerrit Discussion" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to repo-discuss...@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
>
>



--
Jean-Baptiste M. "JBQ" Queru
Technical Lead, Android Open Source Project, Google.

Questions sent directly to me that have no reason for being private
will likely get ignored or forwarded to a public forum with no further
warning.

Brad Larson

unread,
Feb 25, 2013, 2:28:40 PM2/25/13
to repo-d...@googlegroups.com
Sometimes at $DAYJOB we use nested branch names (not sure if 'nested' is the correct term?) such as production/[product-name]/2.00.05 or development/[product-name]/2.01.00.  Will this syntax have any issues with nested branch names?

I assume not, but it also looks like 'draft' and 'publish' are now reserved words and cannot be used as part of a branch name?  (same with any branch name containing '=', but that seems very rare) 

Shawn Pearce

unread,
Feb 25, 2013, 2:53:46 PM2/25/13
to Brad Larson, repo-discuss
No except...

> I assume not, but it also looks like 'draft' and 'publish' are now reserved
> words and cannot be used as part of a branch name? (same with any branch
> name containing '=', but that seems very rare)

Correct.

Mihai Rusu

unread,
Feb 25, 2013, 3:53:10 PM2/25/13
to Shawn Pearce, repo-discuss
On Sun, Feb 24, 2013 at 07:55:36PM -0800, Shawn Pearce wrote:
> https://gerrit-review.googlesource.com/42652
>
> tl;dr: Git won't modify the protocol to let us pass more data. Define
> a new refs/@ prefix that uses options embedded in the reference
> specification:
>
> refs/@master/r=alice
> refs/@master/r=alice/topic=bug42
> refs/@master/r=alice/cc=bob,charlie
>
> My goal is to let `repo upload` pass its --re/--cc flags to Gerrit
> over HTTP where we can't rely on the SSH parser currently used. Its a
> bonus if we can make this definition be "extensible" in the same way
> that command line options are least named and unique.
>
>
> I considered several alternatives:
>
> refs/for/master/bug42%r=alice,cc=bob,charlie
> # splitting on , confuses something like cc

While indeed more convenient to pass a comma separated list another way
is to just repeat the options for multiple values:
refs/for/master/bug42%r=alice,cc=bob,cc=charlie

>
> refs/for/master/bug42%{r=alice}{cc=bob,charlie}
> # fixes the comma issue, but is maybe too ugly
>
> ref/for/master/bug42/r=alice,cc=bob,charlie
> # maybe too ambiguous to parse the options?
>
> refs/for/master/bug42/r=alice/cc=bob,charlie
> # also a bit ugly to parse
>
>
> Something I haven't considered but we may need to is allowing plugins
> to add their own options here. Specifically a commit validator plugin
> might want to let the user say "ignore the validator" e.g. in a case
> where the user is importing a commit done by someone else and the
> validator shouldn't reject its style.
>
> --
> --
> To unsubscribe, email repo-discuss...@googlegroups.com
> More info at http://groups.google.com/group/repo-discuss?hl=en
>
> ---
> You received this message because you are subscribed to the Google Groups "Repo and Gerrit Discussion" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to repo-discuss...@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
>
>

--
Mihai Rusu

Shawn Pearce

unread,
Feb 25, 2013, 8:38:19 PM2/25/13
to Mihai Rusu, repo-discuss
On Mon, Feb 25, 2013 at 12:53 PM, Mihai Rusu <di...@google.com> wrote:
> On Sun, Feb 24, 2013 at 07:55:36PM -0800, Shawn Pearce wrote:
>> I considered several alternatives:
>>
>> refs/for/master/bug42%r=alice,cc=bob,charlie
>> # splitting on , confuses something like cc
>
> While indeed more convenient to pass a comma separated list another way
> is to just repeat the options for multiple values:
> refs/for/master/bug42%r=alice,cc=bob,cc=charlie

And... this is what my latest patch set (ps4, ea630d) now does. Except
the topic is no longer supplied in the branch name part, its now part
of the option suffix. Topic names cannot contain commas, but may
otherwise contain any character. So e.g.:

refs/@refs/meta/config%topic=bugs/internal/1234,r=alice

is legal and parses.

Jason Ganetsky

unread,
Feb 26, 2013, 11:47:43 AM2/26/13
to Shawn Pearce, Mihai Rusu, repo-discuss
Can we use this to work around the No New Changes problem I discussed earlier: https://groups.google.com/forum/?fromgroups=#!topic/repo-discuss/tvLTL3aEKEE

That is, if I try to push a change for review and I get a "No New Changes" error, can I encode the SHA1 hash as an argument in this syntax?


--
--
To unsubscribe, email repo-discuss...@googlegroups.com
More info at http://groups.google.com/group/repo-discuss?hl=en

---
You received this message because you are subscribed to the Google Groups "Repo and Gerrit Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to repo-discuss...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.





--
Jason Ganetsky | Software Engineer | gane...@google.com | US-NYC-9TH 5F117E

Shawn Pearce

unread,
Feb 26, 2013, 11:59:41 AM2/26/13
to Jason Ganetsky, Mihai Rusu, repo-discuss
On Tue, Feb 26, 2013 at 8:47 AM, Jason Ganetsky <gane...@google.com> wrote:
> Can we use this to work around the No New Changes problem I discussed
> earlier:
> https://groups.google.com/forum/?fromgroups=#!topic/repo-discuss/tvLTL3aEKEE
>
> That is, if I try to push a change for review and I get a "No New Changes"
> error, can I encode the SHA1 hash as an argument in this syntax?

I wouldn't encode the SHA1, but we could do something like "force" as
an option e.g.:

refs/@master%force

or a base to make Gerrit work from:

refs/@master%base=$(git rev-parse origin/master)

This latter form is uhm, somewhat dangerous since it could create a
lot of changes if you picked a very old base.

Vladimir Berezniker

unread,
Feb 26, 2013, 12:43:53 PM2/26/13
to repo-d...@googlegroups.com, Jason Ganetsky, Mihai Rusu
Would it make it safer if push required additional parameter with ceiling to the # of expected commits. E.g. refs/@master%base=$(git rev-parse origin/master)%max=3. 


This would also for a related enhancement as to the default meaning of "base" from the current "latest known to Gerrit on any branch" to "latest known on the destination branch".  If one wants to skip the review, one can explicitly set base any any commits that Gerrit have not seen yet, are treated as direct push. 

This would help for those of us that want to allow developers to push their private branches to Gerrit for central storage.

Nasser Grainawi

unread,
Feb 27, 2013, 12:44:08 PM2/27/13
to Shawn Pearce, Mihai Rusu, repo-discuss
I'm still in favor of maintaining /for/ instead of @. Retraining users on @ is just going to be a major PITA.

>
> --
> --
> To unsubscribe, email repo-discuss...@googlegroups.com
> More info at http://groups.google.com/group/repo-discuss?hl=en
>
> ---
> You received this message because you are subscribed to the Google Groups "Repo and Gerrit Discussion" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to repo-discuss...@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
>
>

The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Shawn Pearce

unread,
Feb 28, 2013, 1:20:42 AM2/28/13
to Nasser Grainawi, Mihai Rusu, repo-discuss
On Wed, Feb 27, 2013 at 9:44 AM, Nasser Grainawi <nas...@codeaurora.org> wrote:
> On Feb 25, 2013, at 6:38 PM, Shawn Pearce wrote:
>
>> On Mon, Feb 25, 2013 at 12:53 PM, Mihai Rusu <di...@google.com> wrote:
>>> On Sun, Feb 24, 2013 at 07:55:36PM -0800, Shawn Pearce wrote:
>>>> I considered several alternatives:
>>>>
>>>> refs/for/master/bug42%r=alice,cc=bob,charlie
>>>> # splitting on , confuses something like cc
>>>
>>> While indeed more convenient to pass a comma separated list another way
>>> is to just repeat the options for multiple values:
>>> refs/for/master/bug42%r=alice,cc=bob,cc=charlie
>>
>> And... this is what my latest patch set (ps4, ea630d) now does. Except
>> the topic is no longer supplied in the branch name part, its now part
>> of the option suffix. Topic names cannot contain commas, but may
>> otherwise contain any character. So e.g.:
>>
>> refs/@refs/meta/config%topic=bugs/internal/1234,r=alice
>>
>> is legal and parses.
>
> I'm still in favor of maintaining /for/ instead of @. Retraining users on @ is just going to be a major PITA.

OK, the latest patch set which Edwin just +1d uses refs/for/ instead
of @. So basically the proposal is to just add %options on the
existing refs/for/:

refs/for/master/topic-name%r=alice,cc=bob,cc=charlie

sort of thing.

Matthias Sohn

unread,
Feb 28, 2013, 4:20:47 AM2/28/13
to Shawn Pearce, Nasser Grainawi, Mihai Rusu, repo-discuss
2013/2/28 Shawn Pearce <s...@google.com>
+1, looks like the least intrusive enhancement

--
Matthias 

Thomas Swindells (tswindel)

unread,
Feb 28, 2013, 4:22:43 AM2/28/13
to Shawn Pearce, Nasser Grainawi, Mihai Rusu, repo-discuss
>> I'm still in favor of maintaining /for/ instead of @. Retraining users on @ is just going to be a major PITA.
> OK, the latest patch set which Edwin just +1d uses refs/for/ instead of @. So basically the proposal is to just add %options on the existing refs/for/:
> refs/for/master/topic-name%r=alice,cc=bob,cc=charlie
> sort of thing.
I like that, I think we've managed to get a design which is both a small change from the users experience, a big change in the functionality it can support, easy to read and easy to parse.

Keep up the good work,

Thomas



Alex Blewitt

unread,
Feb 28, 2013, 5:21:57 AM2/28/13
to Matthias Sohn, Shawn Pearce, Nasser Grainawi, Mihai Rusu, repo-discuss
Yes, I prefer keeping the refs/for and using a separator like this. 

As an observation if you did need to support base64 in the future you could do do with a double separator eg

refs/for/master%%cj1hbGljZSxjYz1ib2IsY2M9Y2hhcmxpZQ==

That provides a way of passing values in the properties that don't fit into the valid namespace regex, such as subject=What do you think?

Alex 

Steve Wadsworth (swadswor)

unread,
Feb 28, 2013, 5:33:32 AM2/28/13
to Shawn Pearce, Nasser Grainawi, Mihai Rusu, repo-discuss
>> I'm still in favor of maintaining /for/ instead of @. Retraining users on @ is just going to be a major PITA.
>
>OK, the latest patch set which Edwin just +1d uses refs/for/ instead of @. So basically the proposal is to just add %options on the existing refs/for/:
>
> refs/for/master/topic-name%r=alice,cc=bob,cc=charlie
>
>sort of thing.

+1
This is by far the clearest syntax. I must confess to not really understanding this thread until I saw this post then it all made sense.

Steve.

Vladimir Berezniker

unread,
Feb 28, 2013, 11:14:22 AM2/28/13
to repo-d...@googlegroups.com
For complicated cases as I recall Shawn mentioned Pushing data via annotated tag option.  If base64 encoding is required might as well build a tag object.

Vladimir

Mihai Rusu

unread,
Feb 28, 2013, 3:24:35 PM2/28/13
to Shawn Pearce, Nasser Grainawi, repo-discuss
+1

--
Mihai Rusu
Reply all
Reply to author
Forward
0 new messages