eclipse / omr

Eclipse OMR™ Cross platform components for building reliable, high performance language runtimes
http://www.eclipse.org/omr
Other
939 stars 394 forks source link

Improve new Jenkins pipeline #5067

Open janvrany opened 4 years ago

janvrany commented 4 years ago

Commit 4ab849fefe introduced a new pipeline. However, during testing on my own CI I found a couple issues with it.

  1. Probably the biggest one is that the new pipeline depends on build parameter to decide which target (os + arch) to build. The problem with this is that parametrized builds cannot be used with multibranch pipelines, which I personally use to pre-validate my (RISC-V) changes and PRs.

  2. The build timeout of 1 hour may not be enough for all builds. For example, native RISC-V build on SiFive Unleashed board takes ~1.5 hour. A native RISC-V build inside QEMU VM takes ~6 hours.

  3. Minor and easy-to-fix bug for RISC-V build (will take care of it)

  4. Use of hard-coded workspace names (already discussed in #5020)

  5. The node link in build description is not rendered as HTML by Jenkins, so it displays in verbatim something like <a ​href="http://192.168.27.253:80​80/computer/debian-riscv64">de​bian-riscv64</a>

  6. Linux x86 build uses ninja instead of make to compile. Unless there's a good reason, using make would simplify a little setup of an x86 build slave.

The aim of this issue is to discuss the above issues and agree on solution.

janvrany commented 4 years ago

FYI @AdamBrousseau.

As for 1. I suggest to refactor omrbuild.groovy a little and then change existing pipelines (in https://github.com/eclipse/omr/tree/master/buildenv/jenkins/jobs) to something like:

node {
  ws {
    checkout scm
    pipeline = load "buildenv/jenkins/omrbuild.groovy
    pipeline.run("linux_x86_64")
  }
}

This way we can avoid using parameter and therefore:

  1. It will work with multibranch pipelines
  2. No need to reconfigure existing Jenkins jobs
  3. Simpler setup (no need to define choice parameter with only one value)
  4. One can still have - say - buildenv/jenkins/jobs/Build which can be parametrized and can have various other parameters. Moreover, the pipeline itself may specify parameters (via properties([parameters(...)]) step), not the job.
    
    I'm currently working on this. 
AdamBrousseau commented 4 years ago
  1. I'm curious about multibranch pipelines. We've never used them. What I'm guessing is that since you have your own farm & repo, you use multibranch in order to have builds on any branch you push up to your fork, probably automatically. We've never used this feature because we have so many devs and we use the fork model. Instead we have "personal builds" where a user can specify their fork/branch for a build.

I think the build config is a bit of tomato/potato. I agree the single choice param is silly. I came up with the idea so that all the jobs could pull the same Jenkinsfile to keep less duplicated code across numerous files. I couldn't think of another way to get a value injected into a pipeline.

I'm not against changing it for the better. We could probably set it up so that both ways would work. I'm eager to see what you come up with.

In terms of other params, since traditionally I've used a single top level file, I don't like using properties in the Jenkinsfile. We usually have many different job using the same file so the default params change from job to job. Ssome jobs use params that others don't set and rely on defaults. Not to mentions other properties like triggers or build discarder. But again, I'm not against doing it another/a better way, just giving you my perspective.

  1. Whatever you need, those are just the safe numbers we came up with. We could have different timeouts for different specs but probably not necessary.

  2. I guess this depends on the outcome of #1.

  3. There's a Jenkins setting to fix this. Here's the SO link which will explain it better than I can.

  4. You'll have to talk to dev about that. This is what I got from @rwy0717

    We wanted to test against the ninja generator in cmake, so we moved one build over. The ninja generator tends to be more strict about how compiler commands are formatted. While the makefile generator just munges it’s input and can deal with whatever.

janvrany commented 4 years ago

I think the build config is a bit of tomato/potato. I agree the single choice param is silly. I came up with the idea so that all the jobs could pull the same Jenkinsfile to keep less duplicated code across numerous files. I couldn't think of another way to get a value injected into a pipeline.

I'm not against changing it for the better. We could probably set it up so that both ways would work. I'm eager to see what you come up with.

Okay, I've opened draft PR #5082 to demonstrate what I'm thinking about and to spark a discussion over a (working) code. I'm not yet done with it (sorry) but it already shows the main idea of how to avoid build parameter and still have all the code in single file.

The next idea is to refactor JSON-line SPECs into objects to avoid duplication in the "build specs" itself. It will also help to quickly see how build for given target differs from "standard build". Will push once done.

janvrany commented 4 years ago

@AdamBrousseau , @fjeremic: do you know why Jenkins pipeline for zOS runs tests but does not publish results?

AdamBrousseau commented 4 years ago

Because of ebcdic encoding. I opened a PR to try and covert but it didn't work and I haven't had a chance to investigate. See https://github.com/eclipse/omr/pull/5060

janvrany commented 4 years ago

@Leonardo2718 , @0xdaryl : May I kindly ask you to poke @genie-omr (build all)? Just to see how my experiment works on Eclipse CI. Thanks!

0xdaryl commented 4 years ago

@genie-omr build all