On Sat 29 Dec 2018 at 17:16, Enrico Olivelli <eolive...@gmail.com> wrote:
> Il sab 29 dic 2018, 17:25 Stephen Connolly < > stephen.alan.conno...@gmail.com> > ha scritto: > > > On Sat 29 Dec 2018 at 16:20, Stephen Connolly < > > stephen.alan.conno...@gmail.com> wrote: > > > > > > > > > > > On Sat 29 Dec 2018 at 15:18, Enrico Olivelli <eolive...@gmail.com> > > wrote: > > > > > >> Il sab 29 dic 2018, 15:17 Stephen Connolly < > > >> stephen.alan.conno...@gmail.com> > > >> ha scritto: > > >> > > >> > There is a security issue with building PRs automatically. > > >> > > > >> > I can see about adding PR discovery to the existing ASF gitbox > plugin, > > >> but > > >> > we’d need committers to ok the build and have reviewed the code as > the > > >> PR > > >> > could contain attacks to be run from ASF hardware... which is why we > > >> don’t > > >> > build PRs at present. > > >> > > > >> > > >> Now I have to review and then push to ASF repo and I have to tell to > the > > >> contributor that I will make CI start. > > >> IMHO a good tradeoff is: > > >> - a committer adds a 'test this please' comment, or '@asfbot test this > > >> please' and then a CI job start > > >> - this job updates the status line of the PR, with a link to the logs > > and > > >> the status of the job > > >> > > >> How does it sounds to you? > > > > > > > > > Like it’ll burn through the GitHub api rate limit like crazy. > > > > I did not think we have 100 repos > > > > > > > The committer goes to Jenkins and clicks the build button on the PR job > > > (which is sitting there ready and waiting), done. > > > > > > > Oh and before I forget, clicking build now I’m Jenkins will update the CI > > status in GitHub for the PR to say in progress and then provide the > result > > when available. > > > > > > > Searching through comments on PRs to find commits with a magic comment > > > string that haven’t been built by Jenkins is certainly what would be > > > lovely... but right now it would burn the GitHub rate limit (which is > > 5,000 > > > requests per hour across the whole ASF) right through. > > > > > > > To be clear, the hard part is efficiently finding “missed” comments and > > associating with the commit hash they belong to. We don’t want an > approval > > to allow attack via an update pushed to the PR. So you have to walk all > the > > comments and associate with the commit hash they applied to... gets > tricky. > > > > Yep, I understand, you are right. > > Another option is to have a script to launch: > > import-pr maven-assembly-plugin #567 > > Then the script + Jenkins: > - bind the pr to a JIRA by scanning git log > - push to ASF (changing 'committer') with a standard name (JIRA id) > - start a job > - add a github status line with a link to the logs > - bonus: the job will change the status line with green/red light > > I already have such kind of script in my company (but for > bitbucket/JIRA/Jenkins and not for such a complex system like Maven CI), > but the hard part is the job which has to write the status line > > Enrico > > > > We could maybe hijack approval state... but that allows merging by the > > author. > > > > This part is not clear to me. Only 'commiters' can push to the repo. Nah. Once committers have approved the PR then the PR can be merged by the creator of the PR even if not a committer... at least that’s the default way GitHub PRs work. GitBox *may* be enforcing a stricter permissions model... but I suspect not (because it makes contributions harder... and the goal of gitbox was to make working from GitHub a first class experience) > > > > > > > > > > > > > >> > > >> Enrico > > >> > > >> > > >> > Other projects at ASF probably missed this point in the video series > > >> > chronicling the development of the plugin... > > >> > > > >> > On Sat 29 Dec 2018 at 13:10, Enrico Olivelli <eolive...@gmail.com> > > >> wrote: > > >> > > > >> > > Hervè, > > >> > > This is the plugin > > >> > > > > >> > > > > >> > > > >> > > > https://wiki.jenkins.io/display/JENKINS/GitHub+Branch+Source+Plugin?desktop=true¯oName=unmigrated-inline-wiki-markup > > >> > > > > >> > > I see our "maven-box" is using some special "Scan Apache Hosted > Git > > >> > > Folder Triggers" source > > >> > > (https://builds.apache.org/job/maven-box/configure) > > >> > > In the Jenkins of my company in a "Multibranch Pipeline" I have a > > >> > > "Branch Sources" box with a "GitHub" option which lets me trigger > > >> > > builds by using PRs > > >> > > > > >> > > > > >> > > Enrico > > >> > > > > >> > > Il giorno sab 29 dic 2018 alle ore 13:43 Hervé BOUTEMY > > >> > > <herve.bout...@free.fr> ha scritto: > > >> > > > > > >> > > > Le samedi 29 décembre 2018, 12:40:20 CET Enrico Olivelli a > écrit : > > >> > > > > Il sab 29 dic 2018, 12:37 Mickael Istria <mist...@redhat.com> > > ha > > >> > > scritto: > > >> > > > > > On Sat, Dec 29, 2018 at 12:01 PM Hervé BOUTEMY < > > >> > > herve.bout...@free.fr> > > >> > > > > > > > >> > > > > > wrote: > > >> > > > > > > But in both cases, currently, there is no automatic GitHub > > PR > > >> > > > > > > > >> > > > > > integration: > > >> > > > > > > Maven committers need to create a branch in the official > > >> > > repository to > > >> > > > > > > benefit > > >> > > > > > > from ASF Jenkins build > > >> > > > > > > > >> > > > > > Ah ok, I wasn't aware the GitHub repo was "unofficial" and > > >> couldn't > > >> > > be > > >> > > > > > used > > >> > > > > > to trigger builds. That sucks... > > >> > > > > > > >> > > > > Maven migrated to gitbox so actually github is an official > repo > > >> for > > >> > > Maven. > > >> > > > > I see the same setup in Zookeeper and Bookkeeper and github pr > > >> plugin > > >> > > works > > >> > > > > like a charm (and I partecipated in setting it up) > > >> > > > oh great, that would be nice to have the same setup for Maven > > repos! > > >> > > > > > >> > > > > > > >> > > > > Enrico > > >> > > > > > > >> > > > > > Any idea how we could have GitHub PR reviews without this > > branch > > >> > > creation > > >> > > > > > > > >> > > > > > > by > > >> > > > > > > committers, be it at ASF or somewhere else? > > >> > > > > > > > >> > > > > > Using TravisCI could be a solution. > > >> > > > > > >> > > > > > >> > > > > > >> > > > > > >> > > > > > >> > > > > > >> --------------------------------------------------------------------- > > >> > > > To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org > > >> > > > For additional commands, e-mail: dev-h...@maven.apache.org > > >> > > > > > >> > > > > >> > > > > --------------------------------------------------------------------- > > >> > > To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org > > >> > > For additional commands, e-mail: dev-h...@maven.apache.org > > >> > > > > >> > > -- > > >> > Sent from my phone > > >> > > > >> -- > > >> > > >> > > >> -- Enrico Olivelli > > >> > > > -- > > > Sent from my phone > > > > > -- > > Sent from my phone > > > -- > > > -- Enrico Olivelli > -- Sent from my phone
