[gentoo-dev] Re: Gentoo-hosted code review

[Michael Palimaka]

On 03/11/15 06:24, Dirkjan Ochtman wrote:
> On Mon, Nov 2, 2015 at 2:04 PM, Kristian Fiskerstrand  wrote:
>> The way I see it, keeping review and committing/pushing separate is a
>> good thing, and removes a lot of the concerns about hosting a review
>> platform as it is sufficient with read-access to repositories.
>>
>> Thanks for showing at least one of the alternatives.
> 
> +1.
> 
> FWIW, I think most of the innovation in review UX (insofar as there is
> any) is happening on ReviewBoard, which is another FOSS review-only
> tool, written on top of Django. A Python-based app might also be more
> hackable for the Gentoo dev audience and possibly more
> security-sensible. I know that Mozilla is starting to deploy it quite
> successfully.
> 
> Cheers,
> 
> Dirkjan
> 
> 

I'm a big fan of ReviewBoard, and actually set up an instance against
gx86 a few years ago. Unfortunately there was minimal interest at the
time, and it seems not a great deal more now.

Re: [gentoo-dev] Re: Gentoo-hosted code review

[Alexander Berntsen]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA512

On 02/11/15 14:24, Michael Palimaka wrote:
> Which workflow do you mean? Most features seem optional, allowing 
> people to work as they wish.
It's been a while since I looked at it outside of GHC, so please bear
in mind these things might have changed since then.

The major obstacle for my company is that it doesn't work if you want
to enforce (or even just allow) that author /= committer. Furthermore,
when I used it it broke author dates and GPG signing. Lastly, you
can't review a patch series -- you'll need either to squash the
patches (which is done automatically), or review them independently.

As for the workflow it to some degrees enforces (and certainly is
optimised for), I don't have a link readily available. I could ask
Austin from GHC about it if you are very curious (he's the one who
sent it to me some time ago).

Also, FWIW, I very much dislike arcanist. Herbert from the GHC team is
working on a lite version[0] that I think might end up more pleasant.
(However, it will likely be optimised for GHC.)


[0]  
- -- 
Alexander
berna...@gentoo.org
https://secure.plaimi.net/~alexander
-BEGIN PGP SIGNATURE-
Version: GnuPG v2

iQIcBAEBCgAGBQJWN2yuAAoJENQqWdRUGk8BCDQP/i86L/ACGcBeXoTZJ0PQ7J1J
QX+af+dC9Lgd5STUbhNVJJDnKUFBtnplmdnMslCTqy/ppf1+0yfWEoRST49sEplS
lgZcT2LxdM0NLT/pEH8q/i+NyqoM536s5cSiLupFpTMypquF/SM9qf0g9u3g1LLj
+WiTT1IEuIYoKLx5bzUNPY9ELzagZebD8DZph23hpGwnIBpAQXX34ovhrg1i9J26
mK1W/knyzEV1vb8NW60TCh+RegaN5kFV2PpSDlE0DdyntNntfOr9y17GCpdTcEeY
okyGlltFZ/zscyxWdk/9EhxzzngQCSrwN2xo9JN9Wv5Ay7klzg9zTRs3ZD8PIeDS
w33Bxvevd6ZTl+cAIufMa80zVEWtvSw+HxEJ2LFXVE4X8soWI95sVzfLmSL5Da0M
OO6/h7/pOImKRk7QrIWxqcTt6XgM3iYBFl0HfZijmzicIvh1JL/93KihQJUxV7yf
dLiCZt06dGT4YC8YdAvmsvzTW2FEh0NwnljmarDfu0WCZAfild2/MRti3IlHXFsu
+PE8pkQ+bHAwm03EL1P7l6iR3XYSSnscrQSv4c0QgczcoZV7tm+O2jb2RVAKau6D
eub0BOb5pYwt2VAzOjOrBSzjGRluGxf1RMrlNauWEgk8N181dse6ztr/43IDh00Z
7TtFY6Vqf+ufy25E2jyC
=jJrQ
-END PGP SIGNATURE-

[gentoo-dev] Re: Gentoo-hosted code review

[Michael Palimaka]

On 02/11/15 22:08, Alexander Berntsen wrote:
> It is tailored to Facebook's workflow. Their workflow does not
> coincide with most other people's workflow. It doesn't work for my
> company, and I suspect it won't work for Gentoo either.

Which workflow do you mean? Most features seem optional, allowing people
to work as they wish.

> On the other hand it offers a lot of neat things, so experimenting
> with it is good. Thanks for doing this work, Michael. Hopefully it
> will lead somewhere.
> 
> As an alternative, that I have been meaning to look at, there's
> critic[0]. It might be useful for someone to have a look at it.
> 
> [0]  
> 
> 

I ran a really quick test and it looks really nice for pre-commit
reviews. If we're looking for a "GitHub replacement" however, it seems a
bit light on features.

Re: [gentoo-dev] Re: Gentoo-hosted code review

[Kristian Fiskerstrand]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA512

On 11/02/2015 01:26 PM, Michael Palimaka wrote:
> On 02/11/15 09:07, Michael Orlitzky wrote:
>> On 11/01/2015 12:44 PM, Michael Palimaka wrote:
>>> There's been a lot of discussion about relying on GitHub for 
>>> pull requests and code review and such, so I have set up a 
>>> Phabricator instance against gentoo.git to see how a free 
>>> alternative might work.
>>> 
>>> ...
>>> 
>>> What do you think?
>>> 
>> 
>> Thanks for working on this. I personally didn't like Phabricator 
>> very much when I used it, but I'm glad someone is trying out
>> code review platforms. I could live with it.
>> 
>> The big question for me is, does the apache user have write 
>> access to the gentoo.git repo? If it does, are we all
>> comfortable with allowing a bajillion-line PHP application
>> unchecked access to our repo? That's the serious problem I see
>> with Gerrit, Gitlab and the rest.
> 
> It supports repository hosting, but that is optional. I too prefer 
> the idea of keeping the repo and other tools separate, so the demo 
> is running again plain anongit (and that seems to work fine).
> 
> 

The way I see it, keeping review and committing/pushing separate is a
good thing, and removes a lot of the concerns about hosting a review
platform as it is sufficient with read-access to repositories.

Thanks for showing at least one of the alternatives.

- -- 
Kristian Fiskerstrand
Public PGP key 0xE3EDFAE3 at hkp://pool.sks-keyservers.net
fpr:94CB AFDD 3034 5109 5618 35AA 0B7F 8B60 E3ED FAE3
-BEGIN PGP SIGNATURE-

iQEcBAEBCgAGBQJWN19KAAoJECULev7WN52FvxYH/1KgehSRWz7YOBAKY3X6J1Zl
qLXYiuUZNA7wI4sI4a4u6YCOdfkTeiKGLiUs54ZD8l2Ne2rpxt0sbK+yoOx3nL4D
CbC1Gb2MdyaWrDki/6MaocrjmCN0KuPoGSP2STaJp4Ss+zZNPZsJZe5sPrMlUs71
RiSr+CdpKDfTOhN7ZN96QbpEQPLYZRKviByuvuymS7iL1gWRF/fnISmVbDghXija
fyDSN05WAOSGjNnDrWWtCIcnCf+ledFRPcKuhNhitt60axgXqo7qYKOb6ZGF1dwl
vzFpt5Msvj73Jptey8b1MkgAC7p1ahP2UCcVwq7QYKl95m0Te9obnaZqoZ3o/Sg=
=Mgsu
-END PGP SIGNATURE-

Re: [gentoo-dev] Re: Gentoo-hosted code review

[Dirkjan Ochtman]

On Mon, Nov 2, 2015 at 2:04 PM, Kristian Fiskerstrand  wrote:
> The way I see it, keeping review and committing/pushing separate is a
> good thing, and removes a lot of the concerns about hosting a review
> platform as it is sufficient with read-access to repositories.
>
> Thanks for showing at least one of the alternatives.

+1.

FWIW, I think most of the innovation in review UX (insofar as there is
any) is happening on ReviewBoard, which is another FOSS review-only
tool, written on top of Django. A Python-based app might also be more
hackable for the Gentoo dev audience and possibly more
security-sensible. I know that Mozilla is starting to deploy it quite
successfully.

Cheers,

Dirkjan

[gentoo-dev] Re: Gentoo-hosted code review

[Duncan]

Patrice Clement posted on Mon, 02 Nov 2015 09:33:49 +0100 as excerpted:

> [gerrit]
> 
> Anyway, just my 2 cents on the topic. Have a look and you'll see in
> terms of features, I think it's on a par with Github. And it's open
> source. ;)

FWIW from previous gerrit suggestions...

The problem there is ... java, along with the maintenance and security 
issues it brings when run on a publicly accessible server where java is 
otherwise unnecessary.  (IIRC, at least one infra person said it's a hard 
no on java running on gentoo infra, period, as it simply cannot be done 
correctly and safely with the resources available.  Tho I'm not 100% sure 
IRC on that one.)

#2 problem, as with several code-review products, is the security issue 
of the huge stack of code (regardless of language) on a web server, with 
direct single-user write access to the tree.  If it were a different user 
for each dev account so unconditional write access wasn't a monolithic 
grant...

Now if a one-way repo sync is done to the tree gerrit accesses from 
gentoo-master, not reversed, sandboxing the tree gerrit has access too, 
the problem is lessened to some degree, but of course that dramatically 
lessens the usefulness as well, since the reviewed code must then be 
checked back into the main tree manually.

Which would seem to be one potential positive for phabricator, since at 
least from the bit here in-thread, it appears to be review-only, no 
direct commit access, thereby eliminating at least that security threat.

-- 
Duncan - List replies preferred.   No HTML msgs.
"Every nonfree program has a lord, a master --
and if you use the program, he is your master."  Richard Stallman

Re: [gentoo-dev] Re: Gentoo-hosted code review

[Patrice Clement]

Monday 02 Nov 2015 09:29:48, Duncan wrote :
> Patrice Clement posted on Mon, 02 Nov 2015 09:33:49 +0100 as excerpted:
> 
> > [gerrit]
> > 
> > Anyway, just my 2 cents on the topic. Have a look and you'll see in
> > terms of features, I think it's on a par with Github. And it's open
> > source. ;)
> 
> FWIW from previous gerrit suggestions...
> 
> The problem there is ... java, along with the maintenance and security 
> issues it brings when run on a publicly accessible server where java is 
> otherwise unnecessary.  (IIRC, at least one infra person said it's a hard 
> no on java running on gentoo infra, period, as it simply cannot be done 
> correctly and safely with the resources available.  Tho I'm not 100% sure 
> IRC on that one.)
> 
> #2 problem, as with several code-review products, is the security issue 
> of the huge stack of code (regardless of language) on a web server, with 
> direct single-user write access to the tree.  If it were a different user 
> for each dev account so unconditional write access wasn't a monolithic 
> grant...
> 
> Now if a one-way repo sync is done to the tree gerrit accesses from 
> gentoo-master, not reversed, sandboxing the tree gerrit has access too, 
> the problem is lessened to some degree, but of course that dramatically 
> lessens the usefulness as well, since the reviewed code must then be 
> checked back into the main tree manually.
> 
> Which would seem to be one potential positive for phabricator, since at 
> least from the bit here in-thread, it appears to be review-only, no 
> direct commit access, thereby eliminating at least that security threat.
> 
> -- 
> Duncan - List replies preferred.   No HTML msgs.
> "Every nonfree program has a lord, a master --
> and if you use the program, he is your master."  Richard Stallman
> 
> 

By reading your answer, I'm not sure if it is clear or obvious for most users 
how 
the workflow between the Gentoo infra <-> Github infra functions so maybe we
should explain it one more time:

1) Gentoo developers receive notifications (emails) from Github that somebody's
  sent a PR and would like to merge it into the main repo
2) They go over to github.com/gentoo/gentoo/pulls and checkout the PR
3a) Sometimes (often), the PR has tons of errors and doesn't respect our coding
  standards and suddenly, we get a zillion notifications that mgorny and
  hasufell are peer-reviewing the PR ;)
3b) Sometimes the PR is spot on, the ebuild(s) submitted is (are) flawless, we
  merge it.
4) How do we merge it? We do *NOT* make use of the "Merge" button Github offers
(and never will). Why? Changes would get lost in the wind if we did so, since
the repository sitting on the Github infra is merely a mirror. Instead, we pull
each PR into our own local copy of the Gentoo git repository hosted at
https://gitweb.gentoo.org/repo/gentoo.git/ and then merge them. 

See: https://wiki.gentoo.org/wiki/Gentoo_git_workflow#Workflow_walkthrough and
https://wiki.gentoo.org/wiki/Gentoo_git_workflow#Github_PRs_made_easy

Think of Github as a nice and fancy UI to bridge the gap between developers and
those allergic to a console and used to a browser.

Now, with this explanation out of the way, I'm not sure what you mean by the
"security threat" issue you've brought up your email. 

Gerrit is a code-review web platform that CAN allow commits to be merged, but
this privilege has to be granted to specific users (like Github). It is really
up to us and how infra want to manage merging. It doesn't make Gerrit less or
more secure IMHO. This is a biased argument.

-- 
Patrice Clement
Gentoo Linux developer
http://www.gentoo.org

[gentoo-dev] Re: Gentoo-hosted code review

[Duncan]

Patrice Clement posted on Mon, 02 Nov 2015 11:28:10 +0100 as excerpted:

> By reading your answer, I'm not sure if it is clear or obvious for most
> users how the workflow between the Gentoo infra <-> Github infra
> functions so maybe we should explain it one more time: [...]

> 4) How do we merge it? We do *NOT* make use of the "Merge" button Github
> offers (and never will). Why? Changes would get lost in the wind if we
> did so, since the repository sitting on the Github infra is merely a
> mirror. Instead, we pull each PR into our own local copy of the Gentoo
> git repository hosted at https://gitweb.gentoo.org/repo/gentoo.git/ and
> then merge them.

FWIW I understood the github sequence... but in this particular context 
was discussing gerrit specifically, not github.  But not everyone might 
have understood it, so it doesn't hurt repeating in any case.

> Now, with this explanation out of the way, I'm not sure what you mean by
> the "security threat" issue you've brought up your email.
> 
> Gerrit is a code-review web platform that CAN allow commits to be
> merged, but this privilege has to be granted to specific users (like
> Github). It is really up to us and how infra want to manage merging. It
> doesn't make Gerrit less or more secure IMHO. This is a biased argument.

But... while individual gerrit-registered users get commit grants, it's 
actually the (non-hardened/non-audited java) gerrit instance controlling 
authentication and doing the merges, using the single /system/ gerrit 
user.  The authentication and merges don't go thru the well audited and 
on gentoo infra, hardened, system user authentication, it's the much 
softer (from infra's viewpoint) stack of code, both java runtime and 
gerrit java code, that's doing the merges there, with only the one actual 
system user (the one gerrit runs as) doing the merges into gerrit's copy 
of the git repo.  From the posts I've seen, they'd be far more 
comfortable if the individual commits were by individual system level 
users, authenticated via hardened system authentication, not the stack of 
gerrit/java code with who knows what vulns, running on a publicly 
accessible server where those vulns are to some degree exposed to the 
world as the same gerrit subsystem parsing potentially untrusted world-
submitted content that could trigger those vulns is doing the 
authentication as well.

Tho as I said, if no actual commits could be done via gerrit, so its copy 
of the repo is effectively one-way synced to it from the gentoo master 
repo, similar to the way github's repo copy is only one-way synced from 
the master repo and user submitted commits are pulled to local and 
committed/pushed from there, that would lessen the direct danger to some 
degree.  But that would lessen the utility/convenience to some degree as 
well.

Meanwhile, it would still only deal with one of the two security-related 
factors that it's my understanding have gentoo infra vetoing the gerrit 
idea entirely.  The other one, that they simply don't trust java, and 
gerrit is java based, remains unaddressed, and AFAICT unaddressable.  Tho 
it's possible I read that wrong or remember it wrong.  But to verify that 
either way would have to be done directly by infra, and their attention 
resources are limited, thus my attempt to explain.

But if you look at the back list, doing a search on gerrit, you'll see 
what infra (and others) actually said, unfiltered by my admittedly 
imperfect relaying.  Perhaps that will change things, but I obviously 
don't believe so or I'd not be taking the pains to try to pass on what I 
understood to be their take on things.

OTOH, to this point the biggest gerrit booster has been a non-dev gentoo 
user.  Your boosting as a dev could improve the chances some, but it'll 
still be a pretty tough row to hoe.

The other alternative would be to get someone to donate the hardware and 
third-party administrate an instance for gentoo, bypassing infra for the 
most part as they wouldn't be controlling the hardware, but while gerrit 
is OSS and thus more GSC compliant, such an instance still wouldn't be 
gentoo controlled, and the resources available would be far lower than 
github, so the effective trade would be a small single-instance that's 
likely less longterm reliable but OSS, against the global level but 
proprietary resources of github, so while a different tradeoff, it'd 
hardly be a better or more reliable one.  Which pretty much leaves us 
exactly where we were, with gerrit/java up against a wall of infra 
opposition that's going to be hard to break down.

-- 
Duncan - List replies preferred.   No HTML msgs.
"Every nonfree program has a lord, a master --
and if you use the program, he is your master."  Richard Stallman

[gentoo-dev] Re: Gentoo-hosted code review

[Michael Palimaka]

On 02/11/15 06:23, hasufell wrote:
> On 11/01/2015 06:44 PM, Michael Palimaka wrote:
>> There's been a lot of discussion about relying on GitHub for pull
>> requests and code review and such, so I have set up a Phabricator
>> instance against gentoo.git to see how a free alternative might work.
>>
>> Here's a few examples of how things could work:
>>
>> General post-commit review:
>> http://phabricator.astralcloak.net/rGENTOO27ba62d0c7fcabdc79ce82a064b43d67b3b11cca
>>
>> Tracking commits with issues that need attention:
>> http://phabricator.astralcloak.net/audit/query/open/
>>
>> Pre-commit review: http://phabricator.astralcloak.net/D1
>>
>> Phabricator also has all sorts of fancy (optional) features that could
>> be useful for collaborative development (see http://phabricator.org/ for
>> more info).
>>
>> What do you think?
>>
>>
> 
> phabricator is horrible. I'll definitely use it less (if at all) than
> bugzilla.

Care to elaborate?

[gentoo-dev] Re: Gentoo-hosted code review

[Michael Palimaka]

On 02/11/15 09:07, Michael Orlitzky wrote:
> On 11/01/2015 12:44 PM, Michael Palimaka wrote:
>> There's been a lot of discussion about relying on GitHub for pull
>> requests and code review and such, so I have set up a Phabricator
>> instance against gentoo.git to see how a free alternative might work.
>>
>> ...
>>
>> What do you think?
>>
> 
> Thanks for working on this. I personally didn't like Phabricator very
> much when I used it, but I'm glad someone is trying out code review
> platforms. I could live with it.
> 
> The big question for me is, does the apache user have write access to
> the gentoo.git repo? If it does, are we all comfortable with allowing a
> bajillion-line PHP application unchecked access to our repo? That's the
> serious problem I see with Gerrit, Gitlab and the rest.

It supports repository hosting, but that is optional. I too prefer the
idea of keeping the repo and other tools separate, so the demo is
running again plain anongit (and that seems to work fine).

[gentoo-dev] Re: Gentoo-hosted code review

[Duncan]

Michał Górny posted on Sun, 01 Nov 2015 19:34:06 +0100 as excerpted:

> On Mon, 2 Nov 2015 04:44:39 +1100 Michael Palimaka
>  wrote:
> 
>> There's been a lot of discussion about relying on GitHub for pull
>> requests and code review and such, so I have set up a Phabricator
>> instance against gentoo.git to see how a free alternative might work.
>> 
>> Here's a few examples of how things could work:
>> 
>> General post-commit review:
>> http://phabricator.astralcloak.net/
rGENTOO27ba62d0c7fcabdc79ce82a064b43d67b3b11cca
>> 
>> Tracking commits with issues that need attention:
>> http://phabricator.astralcloak.net/audit/query/open/
>> 
>> Pre-commit review: http://phabricator.astralcloak.net/D1
>> 
>> Phabricator also has all sorts of fancy (optional) features that could
>> be useful for collaborative development (see http://phabricator.org/
>> for more info).
>> 
>> What do you think?
> 
> At a first glance -- terribly unreadable, wtf is all that tiny stuff
> thrown at me all at once? But I guess we can get used to it, or get some
> kind of sane theme. Tiny, gray text on a little brighter gray background
> with some more shades of gray-cyan around doesn't help readability at
> all.

What browser were you using?  Seems reasonable here on firefox, and I 
often enough have problems with web pages that I normally run privoxy 
primarily as a color filter (as well as font size adjuster), switching 
the normal dark text on a glaring white background to light text on a 
dark or black background, without forcing all pages to the same color 
scheme as the normal user-side accessibility style-sheet would do.

I tried both with my privoxy filters active, getting the expected light 
text on dark background (with darker red and darker green where they'd 
normally be light, and black as the default), and with privoxy bypassed, 
which gave me a white background with black or near-black text, except 
for the diffs, etc, which had the expected light red/green backgrounds, 
and links, standard browser link color (here set to cyan unvisited, 
yellow visited), I believe.

And at normal 100% zoom, firefox displayed ordinary sized readable text, 
too, no zooming in/out necessary.

So I'd guess it was either your browser, or the theme was already 
switched (possible I suppose, tho I don't see a post here indicating 
that).

> What's the deal with 'rGENTOO56bd759df1d0'? Can't it be made to use
> normal commit hashes, or at least put some separator in that? I know
> enlightenment people like this kind of stuff but it's neither friendly,
> not readable. And it's going to make copy-paste harder.

++.  That's inconvenient, to say the least.

> Second thought, it's slow. I mean, I open a directory and wait a few
> seconds for detailed information to appear, with my CPU getting hot for
> no good reason. I can only guess how hot the server gets in the
> meantime...

While waiting for the detail did take a bit, I didn't notice my CPU doing 
anything unusual and rendering seemed speedy enough once the detail came 
down, even tho I had been primed by your post to look for it.  Again, 
that was with or without privoxy doing its own parsing and filtering as 
well.  Again, the CPU thing could well be browser specific.

But while for just browsing the delay didn't seem extraordinarily long 
for being served from an Internet server obviously doing some dynamic 
calculation and page generation, actually working with it, I agree, 
waiting for the detail to show up could get old rather fast.

While my home system's reasonably powered for a gentoo build system (I'd 
say nothing special mid-grade given it's a build machine, AMD bulldozer-1-
based fx6100, 6-core, slightly overclocked to 3.6 GHz, 16 gig RAM), the 
systems I use at work are slow enough I press a button and find myself 
wondering if it's just slow to respond, or if it didn't register and I 
need to press it again, and while one does get used to pressing a button 
and waiting... it's definitely nice to be back on my own faster system 
again.  So I know what it's like trying to work with slow responding 
systems for hours at a time, and yes, while one does get used to it, it 
certainly does get old, and I can well imagine this would as well.

But as I've not used any other review systems and just browsed this setup 
a bit, I've nothing to go on in terms of comparison.

(No real opinion on the rest.)

-- 
Duncan - List replies preferred.   No HTML msgs.
"Every nonfree program has a lord, a master --
and if you use the program, he is your master."  Richard Stallman