llvm dev - Jan 2018 - [cfe-dev] [Proposal] Automatically Cc: cfe-commits@ on Clang reviews

Forgot to mention:
> Also, how would arcanist pick up two callsigns here? Wouldn't it just
pick the from the closest surrounding .arcconfig?

The review request will belong to a single repository, as you noticed (from
the closest .arcconfig to where the arc command was invoked).

However, when commits land, they will be imported under either one (rL for
LLVM diffs) or two (rL + rC for e.g. Clang diffs) repositories.

Ben

On Mon, Nov 27, 2017 at 10:00 AM Ben Hamilton <benhamilton at google.com>
wrote:
> Good point, and sorry for the confusion.
>
> Following the monorepo setup instructions
>
<https://llvm.org/docs/GettingStarted.html#for-developers-to-work-with-a-git-monorepo>,
> diffs sent out from a monorepo will use llvm/.arcconfig
> <https://reviews.llvm.org/diffusion/L/browse/llvm/trunk/.arcconfig>.
>
> I haven't committed any changes to that .arcconfig yet, and in addition
> there aren't yet any Herald rules which copy llvm-commits@ on review
> requests sent out to the LLVM project.
>
> So, that explains why llvm-commits@ was not copied on D40312.
>
> The previous setup made sense, since review requests for clang et al were
> also lumped together in the rL LLVM Diffusion repository
> <https://reviews.llvm.org/diffusion/L/>, and we didn't want to
subscribe
> llvm-commits@ on those.
>
> Once I land the set of commits so each project has its own Diffusion
> repository (and we give it a week or so so everyone is up to date), we can
> update llvm/.arcconfig to explicitly set revisions as belonging to the rL
> repository, and then set up a Herald rule to subscribe llvm-commits@ on
> those.
>
> Does that help?
>
> Ben
>
> On Mon, Nov 27, 2017 at 9:42 AM Philip Pfaffe <philip.pfaffe at
gmail.com>
> wrote:
>
>> Hi Ben,
>> (+llvm-dev since the email I'm replying to wasn't sent there)
>>
>> 2017-11-21 17:18 GMT+01:00 Ben Hamilton via cfe-dev <
>> cfe-dev at lists.llvm.org>:
>>
>>> OK. I confirmed that Stephan's process to send out cross-repo
diffs from
>>> the monorepo is not affected by my proposal.
>>>
>>> I'm a bit late to the party, and I might just not have
comprehended this
>> correctly. But does your example actually work? I didn't see any
email
>> about D40312 on llvm-commits, shouldn't there have been one? Also,
how
>> would arcanist pick up two callsigns here? Wouldn't it just pick
the from
>> the closest surrounding .arcconfig?
>>
>>
>>
>> tl;dr: https://reviews.llvm.org/D40312 (which was sent out on top of
>>> D40179 to emulate what happens with a monorepo after my proposal
lands).
>>>
>>> Steps I took (starting with
>>>
https://llvm.org/docs/GettingStarted.html#for-developers-to-work-with-a-git-monorepo
>>> ):
>>>
>>> ==>>> % git clone
https://github.com/llvm-project/llvm-project-20170507/
>>> llvm-project
>>> % cd llvm-project
>>> % cp llvm/.arcconfig .
>>> % mkdir -p .git/info
>>> % echo .arcconfig >> .git/info/exclude
>>> # Manually apply D40179 into the clang-tools-extra directory
>>> % arc export --revision D40179 --git | patch -d clang-tools-extra
-p1
>>> patching file .arcconfig
>>> % git diff
>>> diff --git a/clang-tools-extra/.arcconfig
b/clang-tools-extra/.arcconfig
>>> index f846581763e..d4a00161bce 100644
>>> --- a/clang-tools-extra/.arcconfig
>>> +++ b/clang-tools-extra/.arcconfig
>>> @@ -1,4 +1,4 @@
>>>  {
>>> -  "project_id" : "clang-tools-extra",
>>> +  "repository.callsign" : "CTE",
>>>    "conduit_uri" : "https://reviews.llvm.org/"
>>>  }
>>>  % git commit -a -m "Differential Revision:
>>> https://reviews.llvm.org/D40179"
>>>  % echo "Test 1" >> llvm/README.txt
>>>
>>>  % echo "Test 2" >> clang-tools-extra/README.txt
>>>
>>>  % git commit -a -m "[Test] Monorepo test diff crossing
Differential
>>> repositories"
>>>  % arc diff --create --reviewers sberg HEAD^
>>> ==>>>
>>> Ben
>>>
>>> On Tue, Nov 21, 2017 at 8:00 AM Stephan Bergmann <sbergman at
redhat.com>
>>> wrote:
>>>
>>>> On 11/21/2017 03:41 PM, Ben Hamilton wrote:
>>>> >  > My understanding was that, while there currently is
no official
>>>> > monorepo, it is at least possible and accepted to send
single,
>>>> > cross-repo patches for review.  That would no longer be
possible,
>>>> right?
>>>> >
>>>> > I assume cross-repo patches would be constructed by hand,
right? Or do
>>>> > you mean using the unofficial monorepo from 2016
>>>> > <https://github.com/joker-eph/llvm-project>?
>>>>
>>>> I'm using
<https://github.com/llvm-project/llvm-project-20170507> (as
>>>> advertised at
>>>> <
>>>>
https://llvm.org/docs/GettingStarted.html#for-developers-to-work-with-a-git-monorepo
>>>> >),
>>>> where such a "cross-repo diff" is just a matter of a
plain 'git diff'.
>>>>
>>>> > An example of an existing cross-repo review in
Differential would be
>>>> > nice—do you happen to have one handy?
>>>>
>>>> I just happened to file <https://reviews.llvm.org/D40295>
a moment ago,
>>>> spanning clang and compiler-rt.
>>>>
>>>> > I think either way, we'd keep the ability to send
cross-repo patches
>>>> to
>>>> > the top-level LLVM repo just the same way you could today.
>>>> >
>>>> > I'll confirm that if you can help me construct an
example cross-repo
>>>> > patch for review.
>>>>
>>>> Thanks,
>>>> Stephan
>>>>
>>>
>>> _______________________________________________
>>> cfe-dev mailing list
>>> cfe-dev at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>>>
>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20171127/fb62e096/attachment.html>

It seems like this has already been set up for all the projects except for
libunwind. Can we create a Differential repository for it as well? I'll be
happy to update its .arcconfig once that's done.

On Mon, Nov 27, 2017 at 9:02 AM Ben Hamilton via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> Forgot to mention:
>
>
> > Also, how would arcanist pick up two callsigns here? Wouldn't it
just
> pick the from the closest surrounding .arcconfig?
>
> The review request will belong to a single repository, as you noticed
> (from the closest .arcconfig to where the arc command was invoked).
>
> However, when commits land, they will be imported under either one (rL for
> LLVM diffs) or two (rL + rC for e.g. Clang diffs) repositories.
>
> Ben
>
> On Mon, Nov 27, 2017 at 10:00 AM Ben Hamilton <benhamilton at
google.com>
> wrote:
>
>> Good point, and sorry for the confusion.
>>
>> Following the monorepo setup instructions
>>
<https://llvm.org/docs/GettingStarted.html#for-developers-to-work-with-a-git-monorepo>,
>> diffs sent out from a monorepo will use llvm/.arcconfig
>>
<https://reviews.llvm.org/diffusion/L/browse/llvm/trunk/.arcconfig>.
>>
>> I haven't committed any changes to that .arcconfig yet, and in
addition
>> there aren't yet any Herald rules which copy llvm-commits@ on
review
>> requests sent out to the LLVM project.
>>
>> So, that explains why llvm-commits@ was not copied on D40312.
>>
>> The previous setup made sense, since review requests for clang et al
were
>> also lumped together in the rL LLVM Diffusion repository
>> <https://reviews.llvm.org/diffusion/L/>, and we didn't want
to subscribe
>> llvm-commits@ on those.
>>
>> Once I land the set of commits so each project has its own Diffusion
>> repository (and we give it a week or so so everyone is up to date), we
can
>> update llvm/.arcconfig to explicitly set revisions as belonging to the
rL
>> repository, and then set up a Herald rule to subscribe llvm-commits@ on
>> those.
>>
>> Does that help?
>>
>> Ben
>>
>> On Mon, Nov 27, 2017 at 9:42 AM Philip Pfaffe <philip.pfaffe at
gmail.com>
>> wrote:
>>
>>> Hi Ben,
>>> (+llvm-dev since the email I'm replying to wasn't sent
there)
>>>
>>> 2017-11-21 17:18 GMT+01:00 Ben Hamilton via cfe-dev <
>>> cfe-dev at lists.llvm.org>:
>>>
>>>> OK. I confirmed that Stephan's process to send out
cross-repo diffs
>>>> from the monorepo is not affected by my proposal.
>>>>
>>>> I'm a bit late to the party, and I might just not have
comprehended
>>> this correctly. But does your example actually work? I didn't
see any email
>>> about D40312 on llvm-commits, shouldn't there have been one?
Also, how
>>> would arcanist pick up two callsigns here? Wouldn't it just
pick the from
>>> the closest surrounding .arcconfig?
>>>
>>>
>>>
>>> tl;dr: https://reviews.llvm.org/D40312 (which was sent out on top
of
>>>> D40179 to emulate what happens with a monorepo after my
proposal lands).
>>>>
>>>> Steps I took (starting with
>>>>
https://llvm.org/docs/GettingStarted.html#for-developers-to-work-with-a-git-monorepo
>>>> ):
>>>>
>>>> ==>>>> % git clone
https://github.com/llvm-project/llvm-project-20170507/
>>>> llvm-project
>>>> % cd llvm-project
>>>> % cp llvm/.arcconfig .
>>>> % mkdir -p .git/info
>>>> % echo .arcconfig >> .git/info/exclude
>>>> # Manually apply D40179 into the clang-tools-extra directory
>>>> % arc export --revision D40179 --git | patch -d
clang-tools-extra -p1
>>>> patching file .arcconfig
>>>> % git diff
>>>> diff --git a/clang-tools-extra/.arcconfig
b/clang-tools-extra/.arcconfig
>>>> index f846581763e..d4a00161bce 100644
>>>> --- a/clang-tools-extra/.arcconfig
>>>> +++ b/clang-tools-extra/.arcconfig
>>>> @@ -1,4 +1,4 @@
>>>>  {
>>>> -  "project_id" : "clang-tools-extra",
>>>> +  "repository.callsign" : "CTE",
>>>>    "conduit_uri" :
"https://reviews.llvm.org/"
>>>>  }
>>>>  % git commit -a -m "Differential Revision:
>>>> https://reviews.llvm.org/D40179"
>>>>  % echo "Test 1" >> llvm/README.txt
>>>>
>>>>  % echo "Test 2" >>
clang-tools-extra/README.txt
>>>>
>>>>  % git commit -a -m "[Test] Monorepo test diff crossing
Differential
>>>> repositories"
>>>>  % arc diff --create --reviewers sberg HEAD^
>>>> ==>>>>
>>>> Ben
>>>>
>>>> On Tue, Nov 21, 2017 at 8:00 AM Stephan Bergmann <sbergman
at redhat.com>
>>>> wrote:
>>>>
>>>>> On 11/21/2017 03:41 PM, Ben Hamilton wrote:
>>>>> >  > My understanding was that, while there currently
is no official
>>>>> > monorepo, it is at least possible and accepted to send
single,
>>>>> > cross-repo patches for review.  That would no longer
be possible,
>>>>> right?
>>>>> >
>>>>> > I assume cross-repo patches would be constructed by
hand, right? Or
>>>>> do
>>>>> > you mean using the unofficial monorepo from 2016
>>>>> > <https://github.com/joker-eph/llvm-project>?
>>>>>
>>>>> I'm using
<https://github.com/llvm-project/llvm-project-20170507> (as
>>>>> advertised at
>>>>> <
>>>>>
https://llvm.org/docs/GettingStarted.html#for-developers-to-work-with-a-git-monorepo
>>>>> >),
>>>>> where such a "cross-repo diff" is just a matter
of a plain 'git diff'.
>>>>>
>>>>> > An example of an existing cross-repo review in
Differential would be
>>>>> > nice—do you happen to have one handy?
>>>>>
>>>>> I just happened to file
<https://reviews.llvm.org/D40295> a moment
>>>>> ago,
>>>>> spanning clang and compiler-rt.
>>>>>
>>>>> > I think either way, we'd keep the ability to send
cross-repo patches
>>>>> to
>>>>> > the top-level LLVM repo just the same way you could
today.
>>>>> >
>>>>> > I'll confirm that if you can help me construct an
example cross-repo
>>>>> > patch for review.
>>>>>
>>>>> Thanks,
>>>>> Stephan
>>>>>
>>>>
>>>> _______________________________________________
>>>> cfe-dev mailing list
>>>> cfe-dev at lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>>>>
>>>> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20180110/4f8855d4/attachment.html>

On 10 January 2018 at 21:06, Petr Hosek via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
> It seems like this has already been set up for all the projects except for
> libunwind. Can we create a Differential repository for it as well? I'll
be
> happy to update its .arcconfig once that's done.
Additionally, it might be worth considering if this change means the
guidance should be updated in the docs - they currently recommend
leaving the repository field blank
http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface

Best,

Alex