[LLVMdev] Phabricator update

[Manuel Klimek]

Greetings llvm'ers,

I'm working on an update to phabricator that brings our fork significantly closer to the upstream version, thus making maintenance much easier; this will allow us to upgrade more often.

Upstream has fixed many issues and introduced new ways to customize the email flow, so

a) while rolling out the new version I'll be fiddling with config settings; during that time, you might see emails that are not yet what we'll want the end-state to be; I'll announce on this thread once this is done

b) the new version gives people more control over the emails; that means, if you're a phab user, you might get some emails that the list doesn't get (if you wish to); you can configure that from phab's email settings

Once this is finished, please respond on this thread with your concerns / comments, or file bugs at https://llvm.org/bugs/enter_bug.cgi?product=Website.

Cheers,

/Manuel

[Manuel Klimek]

Unfortunately threading will be broken for changes currently under review (new patches shouldn't have the problem).

I'm very sorry for this inconvenience :(

[Renato Golin]

On 2 July 2015 at 14:01, Manuel Klimek <kli...@google.com> wrote:
> Unfortunately threading will be broken for changes currently under review
> (new patches shouldn't have the problem).
> I'm very sorry for this inconvenience :(

We can live with that. :)

Phab is looking a lot better already, thanks for the hard work!

cheers,
--renato
_______________________________________________
LLVM Developers mailing list
LLV...@cs.uiuc.edu http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

[Keno Fischer]

This might be slightly off topic, but I'd really like a way to be able to run the buildbots off a Phabricator Diff before committing.

[Jonathan Roelofs]

Even further off topic, in phab wishlist land: It'd be awesome if it
were capable of inferring extended patch context by looking at the svn
repo/git mirrors (rather than requiring the person submitting the patch
to re-upload with -U999).


Jon

> <https://urldefense.proofpoint.com/v2/url?u=https-3A__llvm.org_bugs_enter-5Fbug.cgi-3Fproduct-3DWebsite&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=Mfk2qtn1LTDThVkh6-oGglNfMADXfJdty4_bhmuhMHA&m=Fvpa2efUGHSMDfoEJVFnwPMyOv6rPysb__oHeZH6MWI&s=cElWnsknBCk276Ol0-2baMtws6DjzsduR1uVw_4RSOI&e=>.

>
> Cheers,
> /Manuel
>
>
> _______________________________________________
> LLVM Developers mailing list

> LLV...@cs.uiuc.edu <mailto:LLV...@cs.uiuc.edu> http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
>
>
>
> _______________________________________________
> cfe-dev mailing list
> cfe...@cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>

--
Jon Roelofs
jona...@codesourcery.com
CodeSourcery / Mentor Embedded

[Manuel Klimek]

On Thu, Jul 2, 2015 at 6:53 PM Keno Fischer <kfis...@college.harvard.edu> wrote:

This might be slightly off topic, but I'd really like a way to be able to run the buildbots off a Phabricator Diff before committing.

Please file feature requests in the llvm bug tracker :)

[Manuel Klimek]

On Thu, Jul 2, 2015 at 7:27 PM Jonathan Roelofs <jona...@codesourcery.com> wrote:

Even further off topic, in phab wishlist land: It'd be awesome if it
were capable of inferring extended patch context by looking at the svn
repo/git mirrors (rather than requiring the person submitting the patch
to re-upload with -U999).

Yea, this is hard, because detecting which path a patch goes against is hard (for example, if the patch path is a pure add for a new tools/sometool directory, is it in clang or llvm?) and requiring people to do global top-level llvm patches seems rather problematic.

The workaround is to use arc diff (the command line tool).

[Manuel Klimek]

Ok, most of the things should be back to normal - I'm still working on weeding out unhelpful headers (the top-level things that say who did what), let me know if you see ones you find particularly bad from now on (I just switched a bunch off based on the past day of llvm patches).

And a tip: don't set the Revision / Project field in phab - we don't use that for anything, and currently it'll mainly make the emails longer...

Cheers,

/Manuel

[Andrew Wilkins]

On Fri, 3 Jul 2015 at 01:43 Manuel Klimek <kli...@google.com> wrote:

On Thu, Jul 2, 2015 at 7:27 PM Jonathan Roelofs <jona...@codesourcery.com> wrote:

Even further off topic, in phab wishlist land: It'd be awesome if it
were capable of inferring extended patch context by looking at the svn
repo/git mirrors (rather than requiring the person submitting the patch
to re-upload with -U999).

Yea, this is hard, because detecting which path a patch goes against is hard (for example, if the patch path is a pure add for a new tools/sometool directory, is it in clang or llvm?) and requiring people to do global top-level llvm patches seems rather problematic.

The workaround is to use arc diff (the command line tool).

Speaking of which, in case anyone else has issues with arc today, this might save you a minute or two.

$ arc diff

Exception

ERR-CONDUIT-CALL: API Method "differential.query" does not define these parameters: 'arcanistProjects'.

(Run with --trace for a full exception trace.)

Fix this by upgrading with "arc upgrade". 

Then, say you're sending a patch for Clang, you'll need to use "cfe-commits-list" in the subscriber list instead of "cfe-commits". Otherwise you get:

"""

Commit message has errors:

      - Error parsing field "Subscribers": The objects you have listed

      include objects which do not exist (cfe-commits).

"""

(this might be temporary?)

Cheers,

Andrew

LLV...@cs.uiuc.edu         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

[Manuel Klimek]

On Fri, Jul 3, 2015 at 10:02 AM Andrew Wilkins <axw...@gmail.com> wrote:

On Fri, 3 Jul 2015 at 01:43 Manuel Klimek <kli...@google.com> wrote:

On Thu, Jul 2, 2015 at 7:27 PM Jonathan Roelofs <jona...@codesourcery.com> wrote:

Even further off topic, in phab wishlist land: It'd be awesome if it
were capable of inferring extended patch context by looking at the svn
repo/git mirrors (rather than requiring the person submitting the patch
to re-upload with -U999).

Yea, this is hard, because detecting which path a patch goes against is hard (for example, if the patch path is a pure add for a new tools/sometool directory, is it in clang or llvm?) and requiring people to do global top-level llvm patches seems rather problematic.

The workaround is to use arc diff (the command line tool).

Speaking of which, in case anyone else has issues with arc today, this might save you a minute or two.

$ arc diff

Exception

ERR-CONDUIT-CALL: API Method "differential.query" does not define these parameters: 'arcanistProjects'.

(Run with --trace for a full exception trace.)

Fix this by upgrading with "arc upgrade". 

Then, say you're sending a patch for Clang, you'll need to use "cfe-commits-list" in the subscriber list instead of "cfe-commits". Otherwise you get:

"""

Commit message has errors:

      - Error parsing field "Subscribers": The objects you have listed

      include objects which do not exist (cfe-commits).

"""

(this might be temporary?)

Thanks for reporting - this happened as part of the upgrade automatically (mailing lists were migrated to mailing list users). Changed the names back, you should be able to type cfe-commits/llvm-commits/lldb-commits again.

[Tamas Berghammer]

Hi Manual,

Since the Phabricator update I noticed that the username of the reviewer isn't displayed for some comment added to the review (e.g.: http://reviews.llvm.org/D11016). Can you check it out what is causing this issue?

Thanks,

Tamas

[Manuel Klimek]

Those seem to be comments by the original author? Is this a problem?

[Renato Golin]

On 8 July 2015 at 11:31, Manuel Klimek <kli...@google.com> wrote:
> Those seem to be comments by the original author? Is this a problem?

Not always, see http://reviews.llvm.org/D10676 or any other.

I can only see the commenter name if they change some status, like
adding themselves to the list.

--renato

[Manuel Klimek]

Ah, thanks, interesting - investigating

[Manuel Klimek]

Ok, the problems is that in an attempt to make the emails more concise (so we have less clutter at the top where it says "<foo> added a comment") I changed the translation of "<foo> added a comment" to the empty string. The problem with that is that the same translation is apparently also used in the UI. I'll undo that configuration. Let me know if the "<foo> did <bar>" comments get too annoying and I'll figure something out.

[Renato Golin]

Great, thanks!

[Adam Nemet]

Hi Manuel,

Thanks for the update it has some nice new features.  There is one issue I am seeing since the update though.  I am the reviewer on http://reviews.llvm.org/D10386 but I didn’t get any email for the updates since my entry on July 1.  Do you know what’s going on?  We had some issues recently with Phab emails being delayed on Apple mail servers but these emails seems to have completely disappeared.

Thanks,

Adam

[Manuel Klimek]

You were in sendgrid's bounce list (there must have been at least one bounce from your email to the apple servers). Please let me know if other people had similar problems.

[Manuel Klimek]

Ok, found more bounces - removed them all, things should work again...

[Adam Nemet]

On Jul 8, 2015, at 9:16 AM, Manuel Klimek <kli...@google.com> wrote:

You were in sendgrid's bounce list (there must have been at least one bounce from your email to the apple servers). Please let me know if other people had similar problems.

Ah, lovely :(.  Thanks for the quick fix.

[Adam Nemet]

On Jul 8, 2015, at 9:19 AM, Adam Nemet <ane...@apple.com> wrote:

On Jul 8, 2015, at 9:16 AM, Manuel Klimek <kli...@google.com> wrote:

You were in sendgrid's bounce list (there must have been at least one bounce from your email to the apple servers). Please let me know if other people had similar problems.

Ah, lovely :(.  Thanks for the quick fix.

… just to confirm.  I got my first email from Phab.  Thanks again.

Adam

[Manuel Klimek]

aaaaaand you're on the bounce list again. I'm going to delete you again, but I'd be curious what the problem is...

[Manuel Klimek]

I filed a ticket with sendgrid. Since we're paying them money nowadays, I hope they'll answer quickly

[Manuel Klimek]

Ok, apple addresses still bounce without a reason - the sendgrid folks say this is completely controlled by apple.com's mail servers - do you have somebody you can ask?

[Adam Nemet]

Do you have a bounced email, we can look at for clues?

[Adam Nemet]

OK, for now I filed the ticket with a sample email that did reach me.  Please still let me know if you can dig up a bounced email.

[James Y Knight]

It's actually surprising to me that the emails from Phabricator and llvm's svn *DO* get through most of the time.

Both of these fabricate the "From" address to generate an email purportedly be from the author of the svn revision or phabricator comment's email address. While the message is hopefully from the real person behind that email address, those messages are decidedly NOT from that email account. It's not even in the arguably gray area which is mailing lists (forwarding -- with some modifications to subject and body -- an email really written from the "From" account). No, these messages are just fabricated whole-cloth without the knowledge of the sending domain's email system.

So, recipients which run DMARC checks will rightfully detect these emails as illegitimate, and might reject/quarentine/etc them.

(I've no idea if that's actually what's going on with Apple's servers, but an increasing number of domains are using dmarc checks at least as one input of "potentially-spam", if not to outright reject the messages. And it makes sense to do so: that the entire internet can historically write emails that purport to be from me isn't actually a good feature.)

[Manuel Klimek]

On Fri, Jul 10, 2015 at 9:20 PM James Y Knight <jykn...@google.com> wrote:

It's actually surprising to me that the emails from Phabricator and llvm's svn *DO* get through most of the time.

Both of these fabricate the "From" address to generate an email purportedly be from the author of the svn revision or phabricator comment's email address. While the message is hopefully from the real person behind that email address, those messages are decidedly NOT from that email account. It's not even in the arguably gray area which is mailing lists (forwarding -- with some modifications to subject and body -- an email really written from the "From" account). No, these messages are just fabricated whole-cloth without the knowledge of the sending domain's email system.

So, recipients which run DMARC checks will rightfully detect these emails as illegitimate, and might reject/quarentine/etc them.

(I've no idea if that's actually what's going on with Apple's servers, but an increasing number of domains are using dmarc checks at least as one input of "potentially-spam", if not to outright reject the messages. And it makes sense to do so: that the entire internet can historically write emails that purport to be from me isn't actually a good feature.)

Phabricator is using the "I'm an old school mailing list" trick which also all old school mailing lists use. They set the List-Id field. There was an attempt to break them, but that didn't go over so well. 

Does the llvm mailing list support dmarc by now?

[Manuel Klimek]

Another round of removing apple.com bounces. Let me know if you find anything.

[Adam Nemet]

Will do, hoping to hear back from our IT department today...