Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!

Bug 336732

Summary: kernel-2.eclass - Makefile creates $(T).tmp_gas_check when T is set in environment; should be $(T)/.tmp_gas_check instead, and if unset ./.tmp_gas_check.
Product: Gentoo Linux Reporter: Andrey <ahipp0>
Component: EclassesAssignee: Gentoo Kernel Bug Wranglers and Kernel Maintainers <kernel>
Status: RESOLVED TEST-REQUEST    
Severity: normal CC: ahipp0, mgorny, ssuominen
Priority: High    
Version: unspecified   
Hardware: PPC   
OS: Linux   
Whiteboard:
Package list:
Runtime testing required: ---
Attachments: proposed fix
tout.patch
Patch handling unset T case as well
Slightly modified patch
Latest patch after review

Description Andrey 2010-09-10 16:18:07 UTC
kernel-2.eclass changes TOUT to be "$(T).tmp_gas_check" instead of "$(T)/.tmp_gas_check".
This results in creating
"/var/tmp/.../gentoo-sources-.../temp.tmp_gas_check"
instead of
"/var/tmp/.../gentoo-sources-.../temp/.tmp_gas_check".

Reproducible: Always

Steps to Reproduce:
1. emerge -av gentoo-sources  # any sources
2. Ctrl-C after unpack (or wait until emerge completes)

Actual Results:  
A file "/var/tmp/portage/sys-kernel/gentoo-sources-*/temp.tmp_gas_check" exists.


Expected Results:  
A file "/var/tmp/portage/sys-kernel/gentoo-sources-*/temp.tmp_gas_check" must be
"/var/tmp/portage/sys-kernel/gentoo-sources-*/temp/.tmp_gas_check" and thus deleted after emerge completes (with FEATURE="-keepwork -keeptemp").
Comment 1 Andrey 2010-09-10 16:18:36 UTC
Created attachment 246735 [details, diff]
proposed fix
Comment 2 Tom Wijsman (TomWij) (RETIRED) gentoo-dev 2013-04-12 16:35:19 UTC
Created attachment 345398 [details]
tout.patch

Added proper quoting as well to make it more reliable.

This patch will be part of a series of patches that will be sent to the mailing lists and committed all at once to the tree in the near future.
Comment 3 Tom Wijsman (TomWij) (RETIRED) gentoo-dev 2013-04-17 13:53:29 UTC
  17 Apr 2013; Tom Wijsman <TomWij@gentoo.org> kernel-2.eclass:
  Added a warning after the variables that modifying other variables in
  the eclass is not supported, there is a chance that we will not fix
  resulting bugs. Fixes bug #421721.
  Clarify the default DESCRIPTION and make it not use versions, a
  directory with ebuilds that inherit this eclass may contain multiple
  versions and we also don't want to give the impression that a new
  directory needs to made if that's not the case. Fixes bug #445110.
  Clarified which patch depths are used in the normal output and error
  output when applying patches. Fixes bug #436402.
  Made sure .tmp_gas_check is created inside the temp folder, it
  accidentally created temp.tmp_gas_check instead. Fixes bug #336732.
  Make UNIPATCH_DOCS work again, install 0000_README document when
  using genpatches. Fixes bug #301478.
Comment 4 Tom Wijsman (TomWij) (RETIRED) gentoo-dev 2013-04-17 20:58:53 UTC
Your proposed fix would create it in the root directory if T were not set so that is unacceptable, I also have assumed the wrong T in my fix due to some misunderstand; so, we're back to square one.

We need to implement the following functionality implemented in the Makefile:

- If T is set, use $(T)/.tmp_gas_check
- If T is not set, use .tmp_gas_check in the current directory.

I have reverted above commit for now, until we have a better patch implemented.
Comment 5 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2013-04-18 05:35:01 UTC
(In reply to comment #4)
> Your proposed fix would create it in the root directory if T were not set so
> that is unacceptable, I also have assumed the wrong T in my fix due to some
> misunderstand; so, we're back to square one.
> 
> We need to implement the following functionality implemented in the Makefile:
> 
> - If T is set, use $(T)/.tmp_gas_check
> - If T is not set, use .tmp_gas_check in the current directory.

Erm, how T can be unset? it *must* be set in ebuild env...
Comment 6 Samuli Suominen (RETIRED) gentoo-dev 2013-04-18 06:14:52 UTC
(In reply to comment #5)
> Erm, how T can be unset? it *must* be set in ebuild env...

Correct. I believe it's part of PMS.

But it's not the same across pkg_* and src_* functions. Does it have to be for this change?
Comment 7 Tom Wijsman (TomWij) (RETIRED) gentoo-dev 2013-04-18 07:59:02 UTC
(In reply to comment #5)
> Erm, how T can be unset? it *must* be set in ebuild env...

Guys, you fell for the same mistake as I did...

We're talking about "Makefile scope" here, therefore when the user runs `make` inside /usr/src/linux; so, we're not talking about "ebuild scope" or PMS here! I'll correct the summary to avoid further confusion.

Note that the original code had single quotes and places an unprocessed $(T) in the Makefile, as it is supposed to be. We just need to adapt this to place a slash only if T is set, the other option is to make . a default value for T if that is possible in a Makefile.
Comment 8 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2013-04-18 14:47:38 UTC
Makefiles should use ${TMPDIR}, this one's de facto standard, and always exported per the PMS. Potentially with /tmp fallback.
Comment 9 Alice Ferrazzi Gentoo Infrastructure gentoo-dev 2017-04-29 17:32:24 UTC
cannot reproduce.
Comment 10 Andrey 2017-04-29 17:56:43 UTC
(In reply to Alice Ferrazzi from comment #9)
> cannot reproduce.

It's still reproducible: the same TOUT is used in arch/powerpc/Makefile (as of linux-4.10.3), the same sed lines are still in kernel-2.eclass.

Note, that it only affects powerpc, so you need to be cross-compiling for powerpc or running under powerpc.
Comment 11 Alice Ferrazzi Gentoo Infrastructure gentoo-dev 2017-04-29 18:05:41 UTC
yes just noticed that is triggered with ppc from the code
Comment 12 Andrey 2017-04-29 18:36:38 UTC
Created attachment 471294 [details, diff]
Patch handling unset T case as well

Alice, please, find an updated patch which addresses Tom's concern about unset $(T).
Comment 13 Mike Pagano gentoo-dev 2021-09-02 23:22:33 UTC
Created attachment 737200 [details, diff]
Slightly modified patch

Andrey, can you please test this slightly modified version of your patch ?
Comment 14 Andrey 2021-09-05 17:52:20 UTC
> Andrey, can you please test this slightly modified version of your patch ?

Mike, the patch looks good to me!

Unfortunately, I can't easily test it anymore because I don't have PowerPC toolchain/sysroot anymore.
But running sed command manually changes the Makefile as expected.

While trying to test the patch, I found that
it seems any leftovers of .tmp_gas_check got fully removed in kernel 4.20.
So, there's no need (but no harm either) in running sed on kernels 4.20 and newer.
Also, actual use of .tmp_gas_check got removed in 4.20 as well as 4.14.120, 4.19.15 stable kernels.

However, 4.4 and 4.9 series still have it.

Having said that, I'm not actually sure how Portage can now trigger `make` to touch .tmp_gas_check.
The Makefile rule is triggered by "archprepare" target, so maybe it could get called on `emake headers_install` for linux-headers ebuild somehow.


A couple more comments regarding the patch:

>  	# only do this for kernel < 2.6.27 since this file does not exist in later
> 	# kernels

It seems the comment is stale or confusing.

> +	[[ -n ${KV_MINOR} ]] && ver_test ${KV_MAJOR}.${KV_MINOR}.${KV_PATCH} -lt 2.6.27  && \
> +				ppc_makefile="${S}"/arch/ppc/Makefile

Not a big deal, but it seems the indentation for ppc_makefile= line is off by 2 tabs.
Comment 15 Mike Pagano gentoo-dev 2021-09-06 19:12:37 UTC
Created attachment 737950 [details, diff]
Latest patch after review

Thanks for the visual review.
I'v updated the patch but i'm not going to commit it until someone can test.

That said, I'm going to close this bug until an interested party with the proper hardware wants to test.

After that we can commit the patch attached
Comment 16 Andrey 2021-09-07 14:50:16 UTC
> Latest patch after review

Sorry, looks like my previous comment confused it more than helped with the version checks.

Let's probably do it this way:
 1. If kernel version is < 2.6.27, the file to sed is "${S}"/arch/ppc/Makefile
 2. If kernel version is >= 2.6.27 and < 4.20, the file to sed is "${S}"/arch/powerpc/Makefile
 3. If kernel version is >= 4.20, no need to sed at all.


> I'v updated the patch but i'm not going to commit it until someone can test.
>
> That said, I'm going to close this bug until an interested party with the proper hardware wants to test.
> 
> After that we can commit the patch attached


Fair enough.