[gentoo-catalyst] Re: Catalyst pull request

[gentoo-catalyst] Re: Catalyst pull request

[Rick "Zero_Chaos" Farina]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Please use the gentoo-catalyst ML for these.  I have added to the cc
so the proper eyes will see it.

Thanks,
Zero

On 01/29/2014 10:19 AM, Guy Martin wrote:
> Hi team,
> 
> Can you please consider the following commits :
> 
> https://github.com/gmsoft-tuxicoman/catalyst/compare/6046e57...gmsoft?expand=1
>
> 
> 
> Regarding commit 333808b, the second hunk adds --selective=n to
> emerge always merge the kernel. When having two kernels in the
> livecd specs, the first build works fine but the second one fails
> because the sources are removed and not reemerged.
> 
> 
> Thanks, Guy
> 
> 
> 

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJS6X+eAAoJEKXdFCfdEflKGZYP/3T4gG9rQ21hHl2tixi0nvsK
i8w/N29K9CHrj5N59DW/jDAIgD8LnA2UQxsx4xFwpnbThWeLctwSRF0nz1LXEk0D
UClUZYV0AKQJjb0tnd85h2yD5ymmAVKO1a9SZgUIL9MasgmrSb3WqPZ0Iv51hhfA
QtUAZLBSxRa6yl87j4803o5up4RySuoGWq9PM8kOFF62tW1nDIfXCqNfd98PvK9h
EhVPyM7k8HvCWGmi7zupTC8bPWEOoe0Q4M1iau01vy/96HkXKmqCWOmQItR7Qqc4
Ym0c41QwlCswfiMf33Ap5IJrd5SSLMExL2/8KTC/WFvzLho1DHegxMl3ZXZCO+qB
kB9/pX8amyjOzXM7OQOCR/ECJdgJP6HKciopdYjY2ssm91tDo/czKSNSW22FYXi3
nBO6AW7l18w1WAtxfTKILHxskGiv2RJxUgGI+B7X6NIqrcnieeBgNEsWva0VKwPX
Vau6rInI6qavA+RaZ/qcCgAyTs50SdxW995G78bcXEjIbvkuEre30Qp+YfSSaNUK
f12ugo42KuqVa6287KSIfHWu4ez+lXTgpjO0qBrwDeL4Di+PXoe4mEUcIlevPbbF
kCOG+1+FMO7xlG8Z6bFfPPeoGXOzs2RLnMl/mLHBBSR4A4FMUGpYMURtoWAPrASK
eZ8qHuRHPDWJjYTjhmOf
=Uyzk
-----END PGP SIGNATURE-----

Re: [gentoo-catalyst] Re: Catalyst pull request

[W. Trevor King]

[-- Attachment #1: Type: text/plain, Size: 3322 bytes --]

On Wed, Jan 29, 2014 at 05:24:30PM -0500, Rick "Zero_Chaos" Farina wrote:
> Please use the gentoo-catalyst ML for these.  I have added to the cc
> so the proper eyes will see it.

Thanks.  Should I cull catalyst@ and hppa@ from future responses?

> On 01/29/2014 10:19 AM, Guy Martin wrote:
> > https://github.com/gmsoft-tuxicoman/catalyst/compare/6046e57...gmsoft?expand=1

Since the patches aren't inline, I'll respond to the log output:

  $ git log --graph --topo-order --oneline --decorate origin/2.X..gmsoft/gmsoft
  * 333808b (gmsoft/gmsoft) Fix kernel build when kerncache is disabled.
  * a5b80e5 update-modules doens't exists anymore.
  * 0f4c970 Add 'panic=30' to palo's command line.
  * 6046e57 Add the executable bit to all shell script.

I think 6046e57 is useful, but it's already in pending with 70f7cfc
and fcbcea4 [1].  That series has been warmly received by me on the
mailing list, although others have been silent ;).  I think we should
just merge that into master instead of 6046e57.  I don't care if it's
backported to 2.X.

I've never used PALO [2], so I'm agnostic on 0f4c970.  I agree with
dol-sen's #gentoo-releng comment that it would be better to make this
timeout configurable, but since this is code I'll never use, I don't
feel very strongly about it ;).

a5b80e5 looks good to me; I'd merge it into master too.  In general, I
think better error detection in Catalyst's target scripts would be
useful to help catch this sort of thing.

> > Regarding commit 333808b,

I don't understand the first hunk in 333808b:

  --- a/targets/support/kmerge.sh
  +++ b/targets/support/kmerge.sh
  @@ -199,9 +199,10 @@ fi
   [ -e /etc/portage/make.conf ] && \
          echo "USE=\"\${USE} ${clst_kernel_use} \"" >> /etc/portage/make.conf

  +mkdir -p /tmp/kerncache/${clst_kname}
  +
   if [ -n "${clst_KERNCACHE}" ]
   then
  -       mkdir -p /tmp/kerncache/${clst_kname}
          clst_root_path=/tmp/kerncache/${clst_kname} PKGDIR=${PKGDIR} clst_myemergeopts="--quiet --update --newuse" run_merge "${clst_ksource}" || exit 1
          KERNELVERSION=`portageq best_visible / "${clst_ksource}"`
          if [ ! -e /etc/portage/profile/package.provided ]

Why create /tmp/kerncache if kerncache is disabled?

> > the second hunk adds --selective=n to emerge always merge the
> > kernel. When having two kernels in the livecd specs, the first
> > build works fine but the second one fails because the sources are
> > removed and not reemerged.

I'm not entirely sure how this is supposed to work, but I'm not wild
about --selective=n.  Are the two kernels you're adding to the livecd
both from the same package?  I don't see how that could be, but
otherwise I'm not sure why the current invocation doesn't work.  Maybe
there were more details in the original message?  I haven't built a
livecd stage in a while, so I don't have much more than vague
skepticism here.  I'll try and run some local tests this weekend.

Cheers,
Trevor

[1]: http://thread.gmane.org/gmane.linux.gentoo.catalyst/2614
[2]: http://www.pateam.org/parisc-linux-boot/PA-RISC-Linux-Boot-HOWTO/palo.html

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [gentoo-catalyst] Re: Catalyst pull request

[W. Trevor King]

[-- Attachment #1: Type: text/plain, Size: 3199 bytes --]

On Thu, Jan 30, 2014 at 09:56:03AM +0100, Guy Martin wrote:
> On 2014-01-30 00:34, W. Trevor King wrote:
> > On Wed, Jan 29, 2014 at 05:24:30PM -0500, Rick "Zero_Chaos" Farina 
> >> On 01/29/2014 10:19 AM, Guy Martin wrote:
> >> > https://github.com/gmsoft-tuxicoman/catalyst/compare/6046e57...gmsoft?expand=1
> >
> > I think 6046e57 is useful, but it's already in pending with
> > 70f7cfcand fcbcea4 [1]…
>
> This patch just make it easier to develop and test with a git
> branch.  I think it's a good addition and will definitely not break
> anything.

Agreed on both counts.  I'm just recommending the older 70f7cfc and
fcbcea4 over 6046e57.

> > I've never used PALO [2], so I'm agnostic on 0f4c970.  I agree
> > with dol-sen's #gentoo-releng comment that it would be better to
> > make this timeout configurable, but since this is code I'll never
> > use, I don't feel very strongly about it ;).
> 
> The timeout will be configurable when booting. The user always has
> the option to change the value. It's just there because when I
> install a box remotely, I can easily panic the kernel (like ctrl-d
> in the netboot) and I have no way to recover the box.

If you can't set this remotely already, I don't expect a user will be
able to change the value remotely either.  You want to set this in
Catalyst instead, so I think it's fair to support users who want to
change (or disable) the timeout in Catalyst too.

> >> > Regarding commit 333808b,
> > 
> > I don't understand the first hunk in 333808b:
> >
> > …
> > 
> > Why create /tmp/kerncache if kerncache is disabled?
> 
> This directory is used later on in the kmerge.sh script for a few files.
> For example you have :
> /tmp/kerncache/${clst_kname}/${clst_kname}-${clst_version_stamp}.EXTRAVERSION
> /tmp/kerncache/${clst_kname}/${clst_kname}-${clst_version_stamp}.USE
> 
> I wasn't sure about the use of these files so I went for the safe
> route to create the directory. A proper fix might simply be to avoid
> creating these files at all if kerncache isn't used.

The proper fix route sounds better to me ;).

> >> > the second hunk adds --selective=n to emerge always merge the
> >> > kernel. When having two kernels in the livecd specs, the first
> >> > build works fine but the second one fails because the sources
> >> > are removed and not reemerged.
> > 
> > I'm not entirely sure how this is supposed to work, but I'm not
> > wild about --selective=n.  Are the two kernels you're adding to
> > the livecd both from the same package?  …
> 
> Yes they are both from gentoo-sources. On hppa we have to compile a
> 32 and a 64bit kernel, provide them both to palo and it'll decide
> which one to boot base on the machine it's booting. Some hppa box
> can boot only 32 or 64 while some can boot both.

Got it, thanks.  I'll poke around and see if I can come up with
something that works and preserves the @world entry.  Can you point me
towards some example spec files for testing?

Thanks,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [gentoo-catalyst] Re: Catalyst pull request

[Brian Dolbec]

On Thu, 30 Jan 2014 08:46:11 -0800
"W. Trevor King" <wking@tremily.us> wrote:

> On Thu, Jan 30, 2014 at 09:56:03AM +0100, Guy Martin wrote:
> > On 2014-01-30 00:34, W. Trevor King wrote:
> > > On Wed, Jan 29, 2014 at 05:24:30PM -0500, Rick "Zero_Chaos"
> > > Farina 
> > >> On 01/29/2014 10:19 AM, Guy Martin wrote:
> > >> > https://github.com/gmsoft-tuxicoman/catalyst/compare/6046e57...gmsoft?expand=1
> > >
> > > I think 6046e57 is useful, but it's already in pending with
> > > 70f7cfcand fcbcea4 [1]…
> >
> > This patch just make it easier to develop and test with a git
> > branch.  I think it's a good addition and will definitely not break
> > anything.
> 
> Agreed on both counts.  I'm just recommending the older 70f7cfc and
> fcbcea4 over 6046e57.

There isn't going to be any real development work in the stable 2.X
branch.  Just bug fixes until 3.0 is ready for release.  So, it won't
prove very useful in general.  Plus I'm ready to commit the one in
pending for the main development branch.

> 
> > > I've never used PALO [2], so I'm agnostic on 0f4c970.  I agree
> > > with dol-sen's #gentoo-releng comment that it would be better to
> > > make this timeout configurable, but since this is code I'll never
> > > use, I don't feel very strongly about it ;).
> > 
> > The timeout will be configurable when booting. The user always has
> > the option to change the value. It's just there because when I
> > install a box remotely, I can easily panic the kernel (like ctrl-d
> > in the netboot) and I have no way to recover the box.
> 
> If you can't set this remotely already, I don't expect a user will be
> able to change the value remotely either.  You want to set this in
> Catalyst instead, so I think it's fair to support users who want to
> change (or disable) the timeout in Catalyst too.


How about just a general kernel_opts field in the spec

ie.   kernel_opts: panic=30 foo=bar

That makes it possible to add more than just a panic= to the
commandline while still being simple for catalyst.  It is just one
variable.  The help comment above it in the example spec can list the
options recommended.

> 
> > >> > Regarding commit 333808b,
> > > 
> > > I don't understand the first hunk in 333808b:
> > >
> > > …
> > > 
> > > Why create /tmp/kerncache if kerncache is disabled?
> > 
> > This directory is used later on in the kmerge.sh script for a few
> > files. For example you have :
> > /tmp/kerncache/${clst_kname}/${clst_kname}-${clst_version_stamp}.EXTRAVERSION
> > /tmp/kerncache/${clst_kname}/${clst_kname}-${clst_version_stamp}.USE
> > 
> > I wasn't sure about the use of these files so I went for the safe
> > route to create the directory. A proper fix might simply be to avoid
> > creating these files at all if kerncache isn't used.
> 
> The proper fix route sounds better to me ;).

Yes, if this is to go into the stable branch. It should be done
properly.

> 
> > >> > the second hunk adds --selective=n to emerge always merge the
> > >> > kernel. When having two kernels in the livecd specs, the first
> > >> > build works fine but the second one fails because the sources
> > >> > are removed and not reemerged.
> > > 
> > > I'm not entirely sure how this is supposed to work, but I'm not
> > > wild about --selective=n.  Are the two kernels you're adding to
> > > the livecd both from the same package?  …
> > 
> > Yes they are both from gentoo-sources. On hppa we have to compile a
> > 32 and a 64bit kernel, provide them both to palo and it'll decide
> > which one to boot base on the machine it's booting. Some hppa box
> > can boot only 32 or 64 while some can boot both.
> 
> Got it, thanks.  I'll poke around and see if I can come up with
> something that works and preserves the @world entry.  Can you point me
> towards some example spec files for testing?
> 
> Thanks,
> Trevor
> 

If you could address the above issues, I think the rest of the team
would approve the changes for the 2.X branch.  I can also add them to
the pending branch queue for merge into master.  I have done very
little to the kernel code areas, so there should be few to no conflicts
with the rewrite changes.

Brian Dolbec

Re: [gentoo-catalyst] Re: Catalyst pull request

[W. Trevor King]

[-- Attachment #1: Type: text/plain, Size: 347 bytes --]

On Thu, Jan 30, 2014 at 12:40:07PM -0800, Brian Dolbec wrote:
> How about just a general kernel_opts field in the spec
> 
> ie.   kernel_opts: panic=30 foo=bar

+1.

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]