All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Kevin Hilman" <khilman@baylibre.com>
To: Ana Guerrero Lopez <ana.guerrero@collabora.com>
Cc: kernelci@groups.io
Subject: Re: [kernelci] proposal to build Debian images for every test suite (pull #24/kernelci-build-staging)
Date: Wed, 09 May 2018 18:15:44 -0700	[thread overview]
Message-ID: <7hmux8wbsv.fsf@baylibre.com> (raw)
In-Reply-To: <8b6f78ca-17bb-45bf-91c7-0a52a8e5d875@collabora.com> (Ana Guerrero Lopez's message of "Wed, 9 May 2018 02:00:37 +0200")

"Ana Guerrero Lopez" <ana.guerrero@collabora.com> writes:

> I have made a new pull request (PR#25), more details below.

Thanks, I've made a few comments there.

> On 08/05/18 01:09, Kevin Hilman wrote:
>> "Ana Guerrero Lopez" <ana.guerrero@collabora.com> writes:
>
>> Also, at first glance, the shared libs need some review.  The first
>> thing that looks strange is the arch mappings:
>>
>>     def debian_arch =  ["arm64": "armhf",
>>                         "armel": "arm64",
>>                         "x86": "i386",
>>                         "x86_64": "amd64"]
>>
>> It looks to me like the arm64 and armhf are swapped.  The same looks
>> true for 'toolchain_arch'.
>
> My bad, I had this fixed locally this but I never pushed the commit.
>
> I have changed armel to arm to be more consistent with the linux/arch/XX
> naming. If you happen to be using already armel in kernelci, please tell
> me and I'll rename it back to armel.

We're already using armel in kernelCI on purpose, because we still have
a handful of ARMv5 boards.  As I suggested on your PR, maybe we could
build armel and armhf so both are available.

>>> However, if the plan is to have all the Jenkinsfile in the same git
>>> repository, then having an external repository for the shared libraries
>>> is probably a bad idea.
>>
>> I'm not sure what Jenkins best practices are here, but for kernelCI,
>> keeping them separate (at least in the long term) sounds like a good
>> idea.  However, in order to get the structure of the first few jobs
>> agreed upon and merged, it might make sense to keep them all in the same
>> repo for ease of review.
>
> Yeah, I agree. It's everything in the same repository now.
>
>> Also, Tomeu seems to have separate project for building a tiny
>> ramdisk/initramfs that doesn't use the shared libs at all.
>
> For only one job it makes sense to have it all in the same Jenkinsfile.
> But soon I'm expecting to have very similar jobs and we'll end up
> copy-pasting the same blocks of code.
>
> Anyway, for making things easier, right now there is only a function
> (buildImage.groovy) and I have merged there the code from
> getDockerArgs.groovy
> and setPipelineVersion.groovy
> We'll see later when we add more pipelines which functions can be reused
> and need to be split in different files for re-use.

Thanks, that makes it easier to review all together, and doesn't have a
dependency on an external repo for the libs.

> You can see I kept the 3 lines in the top of Jenkinsfile_basic importing
> the library from the same repository. This step is necessary unless
> we declare the library in Jenkins (globally or when creating the pipeline).
> This article has a example very similar to what we're doing and
> explain how to do it:
> https://dev.to/jalogut/centralise-jenkins-pipelines-configuration-using-shared-libraries
> We don't need to do it now, but I wanted to mention it because most
> of the examples you'll find use this method and use the call @Library..
> in the Jenkinsfile.

OK, when we start doing it that way, please include that in link in the
changelog as background.

And, while you're fixing the changelog, s/var/vars/.  (I couldn't figure
out how to comment on the changelog in github.)

>> IMO, what I think would be very helpful at least for initial review and
>> discussion, is to see an initial PR that only has the "basic" build, and
>> ideally also generates a minimal, tiny ramdisk from the same build
>> (e.g. with 'update-initramfs -c -k min' )
>
> The new pull request has only the basic build. I have modified the debos
> files to create the tiny ramdisk and the Jenkinsfile to store it.

Very nice, thank you.

Kevin

  reply	other threads:[~2018-05-10  1:15 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-07 11:06 proposal to build Debian images for every test suite (pull #24/kernelci-build-staging) Ana Guerrero Lopez
2018-05-07 23:09 ` [kernelci] " khilman
2018-05-08  6:11   ` tomeu.vizoso
2018-05-10  1:04     ` Kevin Hilman
2018-05-10  6:56       ` Tomeu Vizoso
2018-05-10 15:41         ` Kevin Hilman
2018-05-11  5:23           ` Tomeu Vizoso
2018-05-16 16:46             ` Ana Guerrero Lopez
2018-05-23  8:21               ` Tomeu Vizoso
2018-05-09  0:00   ` Ana Guerrero Lopez
2018-05-10  1:15     ` Kevin Hilman [this message]
2018-05-16 15:56       ` Ana Guerrero Lopez

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7hmux8wbsv.fsf@baylibre.com \
    --to=khilman@baylibre.com \
    --cc=ana.guerrero@collabora.com \
    --cc=kernelci@groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.