nvidia-drivers-319.17> * Determining the location of the kernel source code nvidia-drivers-319.17> * Found kernel source directory: nvidia-drivers-319.17> * /usr/src/linux * ACCESS DENIED: unlinkat: /.234832.tmp [...] nvidia-drivers-319.17> * Found kernel object directory: nvidia-drivers-319.17> * /lib/modules/3.9.0-gentoo/build nvidia-drivers-319.17> * Found sources for kernel version: nvidia-drivers-319.17> * 3.9.0-gentoo nvidia-drivers-319.17> * Checking for suitable kernel configuration opt [ ok ] nvidia-drivers-319.17> * --------------------------- ACCESS VIOLATION SUMMARY --------------------------- nvidia-drivers-319.17> * LOG FILE: "/var/log/sandbox/sandbox-234644.log" nvidia-drivers-319.17> * nvidia-drivers-319.17> VERSION 1.0 nvidia-drivers-319.17> FORMAT: F - Function called nvidia-drivers-319.17> FORMAT: S - Access Status nvidia-drivers-319.17> FORMAT: P - Path as passed to function nvidia-drivers-319.17> FORMAT: A - Absolute Path (not canonical) nvidia-drivers-319.17> FORMAT: R - Canonical Path nvidia-drivers-319.17> FORMAT: C - Command Line nvidia-drivers-319.17> nvidia-drivers-319.17> F: unlinkat nvidia-drivers-319.17> S: deny nvidia-drivers-319.17> P: //.234832.tmp nvidia-drivers-319.17> A: /.234832.tmp nvidia-drivers-319.17> R: /.234832.tmp nvidia-drivers-319.17> C: rm -f //.234832.tmp //.234832.o [...] The problem is that getfilevar() gets called which in turn writes to the filesystem which is forbidden by PMS (p.39). This is with kernel 3.9 and the paludis PM. Setting M=$T instead of M=$S in line 186 helped, even though $T doesn't have to be defined and access to the FS isn't necessarily granted at all. The proper solution is probably to fallback to getfilevar_noexec if PHASE=pkg_pretend.
I can confirm that setting M=${T} instead of M=${S} does make the violations go away. But according to https://dev.gentoo.org/~ulm/pms/5/pms.html#x1-960009.1.2: pkg_pretend must not write to the filesystem. So, the above seems to be more of a workaround, rather than the correct solution.
Further data: As Tiziano pointed out in #1, the offending function is getfilevar in linux-info.eclass, which calls emake, which then calls make, inside which things go awry. Grepping /usr/portage reveals that getfilevar is *only* pulled in by: get_makefile_extract_function() { local a='' b='' mkfunc='getfilevar' a="$(getfilevar VERSION ${KERNEL_MAKEFILE})" b="$(getfilevar_noexec VERSION ${KERNEL_MAKEFILE})" [[ "${a}" != "${b}" ]] && mkfunc='getfilevar_noexec' echo "${mkfunc}" } which is used in get_version, which is then used in a host of other functions. (NB: kernel-2.eclass has its own similar version of getfilevar.) Comment to getfilevar_noexec says: # If the variable is defined, you will run into problems. See getfilevar for those cases. However, looking at the above get_makefile_extract_function code, if the output of getfilevar and getfilevar_noexec differ, getfilevar_noexec gets picked over getfilevar. In other words, in those special cases the getfilevar function ends up not being used. Does that not obviate its purpose? Or did I miss something? -TJ
Should I go ahead and submit a patch against linux-info.eclass?
*** Bug 499138 has been marked as a duplicate of this bug. ***
Created attachment 368664 [details, diff] fall back to getfilevar_noexec in pkg_pretend phase According to https://dev.gentoo.org/~ulm/pms/5/pms.html#x1-960009.1.2 pkg_pretend must not write to the filesystem. So, in pkg_pretend phase, we avoid calling getfilevar, which can potentially write to the filesystem, thus casing sandbox violations.
Timothy's patch fixes the issue for me. CC'ing QA because this is getting nowhere.
(In reply to Tiziano Müller from comment #0) > * ACCESS DENIED: unlinkat: /.234832.tmp Just to be sure, can you verify that this is not bug #488492? > The problem is that getfilevar() gets called which in turn writes to the > filesystem which is forbidden by PMS (p.39). Are you sure it does? Does a strace reveal make doing this? > This is with kernel 3.9 and the paludis PM. Is this reproducible for you on kernel 3.14 with Portage? I can't reproduce it. > Setting M=$T instead of M=$S in line 186 helped, even though $T doesn't have > to be defined and access to the FS isn't necessarily granted at all. Neither points to /; so, I don't see why this is wrong or changing it would help. > The proper solution is probably to fallback to getfilevar_noexec if > PHASE=pkg_pretend. This beats the purpose of get_makefile_extract_function; probably will want to patch it in get_version instead, if it still reproducible on 3.14 with Portage.
Created attachment 375020 [details, diff] linux-info_get_version.patch Okay, we've had a talk on IRC, filed a bug (see See Also); either your version or this version should fix this, we just need to see where to best place it.
(In reply to Tom Wijsman (TomWij) from comment #9) I took a look at these in a new unit of time and I am seeing a few design flaws/limitations (given my limited understanding of the portage microcosm). This commit http://sources.gentoo.org/cgi-bin/viewvc.cgi/gentoo-x86/eclass/linux-info.eclass?r1=1.49&r2=1.50 added a limitation to the usability of the getfilevar function. According to https://dev.gentoo.org/~ulm/pms/5/pms.html#x1-11800011.1 (and the Gentoo Dev Manual) S is only used in src_* functions. So, the above commit confined the getfilevar's usage to compile and install phases. Perhaps this function should be modified to use ${S} when called from src_* phases and fall back to ${T} in other phases?
Created attachment 376574 [details, diff] patch getfilevar function
I also realized that neither my earlier patch nor TomWij's patch from #9 actually work correctly. The get_version function in linux-info.eclass uses getfilevar/getfilevar_noexec to extract the value of KBUILD_OUTPUT from the Makefile and if you look at the relevant section of the Makefile: ---snip----------------------- # kbuild supports saving output files in a separate directory. # To locate output files in a separate directory two syntaxes are supported. # In both cases the working directory must be the root of the kernel src. # 1) O= # Use "make O=dir/to/store/output/files/" # # 2) Set KBUILD_OUTPUT # Set the environment variable KBUILD_OUTPUT to point to the directory # where the output files shall be placed. # export KBUILD_OUTPUT=dir/to/store/output/files/ # make # # The O= assignment takes precedence over the KBUILD_OUTPUT environment # variable. # KBUILD_SRC is set on invocation of make in OBJ directory # KBUILD_SRC is not intended to be used by the regular user (for now) ifeq ($(KBUILD_SRC),) # OK, Make called in directory where kernel src resides # Do we want to locate output files in a separate directory? ifeq ("$(origin O)", "command line") KBUILD_OUTPUT := $(O) endif ifeq ("$(origin W)", "command line") export KBUILD_ENABLE_EXTRA_GCC_CHECKS := $(W) endif # That's our default target when none is given on the command line PHONY := _all _all: # Cancel implicit rules on top Makefile $(CURDIR)/Makefile Makefile: ; ifneq ($(KBUILD_OUTPUT),) # Invoke a second make in the output directory, passing relevant variables # check that the output directory actually exists saved-output := $(KBUILD_OUTPUT) KBUILD_OUTPUT := $(shell cd $(KBUILD_OUTPUT) && /bin/pwd) $(if $(KBUILD_OUTPUT),, \ $(error output directory "$(saved-output)" does not exist)) ---snip----------------------- you will see that KBUILD_OUTPUT cannot be extracted using getfilevar_noexec. In our particular example it would return: KBUILD_OUTPUT="$(O) $(shell cd $(KBUILD_OUTPUT) && /bin/pwd)" which I have a feeling is not exactly what one would expect. ;) In the end the KV_OUT_DIR does get set correctly to point to KV_DIR, but at the expense of several unnecessary calculations and access violations. <pause> Now, let's take another step back. If you read the comments in the above snipped section, it says that KBUILD_OUTPUT is either set from the env var or from the O= option. And since we are not passing the O= option inside the getfilevar, it will always be set from the environment. An earlier part of the get_version already sets the OUTPUT_DIR from KBUILD_OUTPUT env var (line 491), so there is no need to check the Makefile at all. Or did I miss something? In other words, the section of the get_version that calls get_makefile_extract_function (lines 496-503) along with the get_makefile_extract_function itself are unnecessary and can be dispensed with.
Created attachment 376580 [details, diff] Don't check Makefile for KBUILD_OUTPUT
bump
Just FYI: Quite prominent ebuilds are hit by this issue... * sys-apps/systemd-2.0.8-r3 * apps-emulation/qemu-2.0.0 * x11-drivers/xf86-input-synaptics-1.7.4 * ...
Can somebody please apply the last 2 patches to the linux-info.eclass, so we can close this bug? I've been able to compile the offending packages (x86-input-{evdev,synaptics}, systemd, nvidia-drivers, etc) against the patched version of linux-info.eclass and everything is working with no problems. Tom, any input on this?
(speaking as somebody that wrote lots of linux-info.eclass) I'm not convinced that the patches from comment #11 and comment #13 fix it for all systems, esp. those with separate kernel source & build trees. And moreso, why doesn't it fail on my system? You also never answered the question if it's reproducible with Portage instead of Paludis.
(In reply to Robin Johnson from comment #17) > And moreso, why doesn't it fail on my system? You also never answered the > question if it's reproducible with Portage instead of Paludis. It's not reproducible with portage, because it contains an even number of bugs, which cancel each other out. ;) On a serious note, please consider the following foo-0.ebuild: EAPI=5 inherit linux-info SLOT="0" KEYWORDS="amd64" pkg_pretend() { einfo "S=${S}" [[ -d "${S}" ]] || ewarn "S does not exist yet!" einfo "T=${T}" [[ -d "${T}" ]] || ewarn "T does not exist yet!" kernel_is ge 3 15 } It installs just fine with emerge, but will cause access violations with paludis. The difference is that when run with emerge the variable S is set during the pkg_pretend phase, even though the directory pointed to by S does not yet exist. When kernel_is is called, it ends up calling getfilevar, which calls emake as follows: nonfatal emake -C "${basedname}" M="${S}" ${BUILD_FIXES} -s -f - 2>/dev/null where S points to a non-existent directory. Due to some magic (or an even number of bugs) within the kernel's Makefile, it ends up not creating any temporary files, but still giving the right answer (version components), when called with M=/path/to/non/existent/dir. That's what the patch from comment #11 addresses. It tells getfilevar to pass M=${S} to emake during src_* phases and M=${T} in other phases. It has no bearing on whether the kernel source and build trees are in different dirs. The above patch makes most of the packages using linux-info.eclass happily compile with paludis. So, could we please commit the patch from #11?
The reason I introduced patch in comment #13 is that according to: http://dev.gentoo.org/~ulm/pms/head/pms.html#x1-960009.1.2 the ebuild must not write to the file system in pkg_pretend, not even the ${T} directory. If you look at linux-info.eclass, you will notice that the only function ever calling getfilevar is the get_makefile_extract_function. And the only place that calls that function is the portion of the get_version function, which tries to extract KBUILD_OUTPUT from the kernel Makefile. Look at the lines 488-503 of linux-info.eclass or see below: # OK so now we know our sources directory, but they might be using # KBUILD_OUTPUT, and we need this for .config and localversions-* # so we better find it eh? # do we pass KBUILD_OUTPUT on the CLI? local OUTPUT_DIR=${KBUILD_OUTPUT} # keep track of it KERNEL_MAKEFILE="${KV_DIR}/Makefile" if [[ -z ${OUTPUT_DIR} ]]; then # Decide the function used to extract makefile variables. local mkfunc=$(get_makefile_extract_function "${KERNEL_MAKEFILE}") # And if we didn't pass it, we can take a nosey in the Makefile. OUTPUT_DIR=$(${mkfunc} KBUILD_OUTPUT "${KERNEL_MAKEFILE}") fi And if you also take a look at the snippet of the kernel Makefile in comment #12, you will notice that the only time the KBUILD_OUTPUT is set inside the Makefile is when it is passed to make on the command line. So, in other words, there is no need to "take a nosey in the Makefile" for KBUILD_OUTPUT, is there? So, what the patch in #13 does is it removes the last if/fi along with get_makefile_extract_function. It should probably also remove getfilevar, as it is not used anywhere else. If you think it should do that, I add it to the patch. Robin?
(In reply to Timothy Jones from comment #18) > (In reply to Robin Johnson from comment #17) > So, could we please commit the patch from #11? I've figured out why the problem exists. When most of linux-info was written, pkg_pretend didn't exist yet; and the earliest you were expected to run those functions was pkg_setup, wherein doing stuff to $S is valid. At the same time over the years, the kernel build system itself has evolved, and writes stuff to disk more now :-(. Going back to an old 2.6 kernel, it doesn't seem to write any tempfiles here at all. At the same time, blocking M= in pkg_(pre|post)(inst|rm) and pkg_setup raises red flags to me, because I've seen code there invoking kernel-2/linux-info stuff. So I'm going to commit the following more conservative version, if you confirm that works on Paludis (manually written diff on top of your prior patch in comment #11) ... - local ERROR basefname basedname myARCH="${ARCH}" M="${T}" + local ERROR basefname basedname myARCH="${ARCH}" M="${S}" ... -[[ ${EBUILD_PHASE_FUNC} == src_* ]] && M="${S}" +case ${EBUILD_PHASE_FUNC} in + pkg_info) M="${T}" ;; + pkg_nofetch) M="${T}" ;; + pkg_pretend) M="${T}" ;; +esac ... (In reply to Timothy Jones from comment #19) I'm tracing usages of KBUILD_OUTPUT in other makefiles; it looks like it might be obsolete since some time in the 3.x kernel series. Here's the commit that introduced that check originally, note the date of 2004/11/28: http://sources.gentoo.org/cgi-bin/viewvc.cgi/gentoo-x86/eclass/linux-info.eclass?r1=1.3&r2=1.4& So in 3.something kernels, if you are using separate src/obj directories, the objdir contains a reference back to the srcdir. In older kernels, the srcdir could contain a reference out to the objdir instead. This code would potentially be broken by patch in comment #13. I'm trying to work out if that code is still relevant in any cases, or if we can indeed just get rid of it. getfilevar is still used by linux-2.eclass, but it has a duplicate copy now as well. One of the remaining interesting uses of getfilevar is checking for Makefile problems, but that seems to be obsolete as well.
(In reply to Robin Johnson from comment #20) > So I'm going to commit the following more conservative version, if you > confirm that works on Paludis (manually written diff on top of your prior > patch in comment #11) > ... > - local ERROR basefname basedname myARCH="${ARCH}" M="${T}" > + local ERROR basefname basedname myARCH="${ARCH}" M="${S}" > ... > -[[ ${EBUILD_PHASE_FUNC} == src_* ]] && M="${S}" > +case ${EBUILD_PHASE_FUNC} in > + pkg_info) M="${T}" ;; > + pkg_nofetch) M="${T}" ;; > + pkg_pretend) M="${T}" ;; > +esac > ... Works good! And I am tracking on all the history behind it. :) That's one of the things I am lacking, being a portage newb. BTW, compressing case/esac into a one-liner, eg: pkg_info|pkg_nofetch|pkg_pretend) M="${T}" ;; also works like a charm. :) Do you want me to resubmit the patch or you are good from here?
Created attachment 377912 [details, diff] Revised patch for getfilevar, but with a one-liner (just cuz I am stingy)
(In reply to Robin Johnson from comment #20) > +case ${EBUILD_PHASE_FUNC} in > + pkg_info) M="${T}" ;; > + pkg_nofetch) M="${T}" ;; > + pkg_pretend) M="${T}" ;; > +esac This doesn't look right. These functions must not write to the filesystem at all.
Also, according to PMS, S is valid in src_* only. (In practice, the directory is created in src_unpack, so it doesn't exist in pkg_pretend or pkg_setup.)
(In reply to Ulrich Müller from comment #23) > (In reply to Robin Johnson from comment #20) > > +case ${EBUILD_PHASE_FUNC} in > > + pkg_info) M="${T}" ;; > > + pkg_nofetch) M="${T}" ;; > > + pkg_pretend) M="${T}" ;; > > +esac > > This doesn't look right. These functions must not write to the filesystem at > all. Have you read the earlier comments? Can you submit a better version, please?
If the function cannot work without writing to the filesystem, then it must not be called in any of the pretend, info, or nofetch phases (but from pkg_setup() instead, for example). Alternatively, we could loosen up the specification and allow write access to ${T} and ${TMPDIR} in all phases. I had already suggested that in 2011, but it went nowhere at the time: https://archives.gentoo.org/gentoo-dev/msg_ea6dce57c39ff597afdca7ba74f7cc73.xml Let's please fix this a controlled way, not by switching from one workaround in the eclass to another one.
(In reply to Ulrich Müller from comment #26) > If the function cannot work without writing to the filesystem, then it must > not be called in any of the pretend, info, or nofetch phases (but from > pkg_setup() instead, for example). > > Alternatively, we could loosen up the specification and allow write access > to ${T} and ${TMPDIR} in all phases. I had already suggested that in 2011, > but it went nowhere at the time: > https://archives.gentoo.org/gentoo-dev/msg_ea6dce57c39ff597afdca7ba74f7cc73. > xml > > Let's please fix this a controlled way, not by switching from one workaround > in the eclass to another one. Ulrich, I agree with you totally and want to do it the right way. Few things to consider: 1. Currently linux-info.eclass "works" for portage by means of 2 complimentary bugs: (a) portage sets S to point to a non-existent dir in pkg_pretend and calls getfilevar, which in turn calls make with M=/path/to/non/existent/dir, which causes it not to create any temp files; and (b) apparently portage allows ebuilds to arbitrarily write to the file system in pkg_pretend (see my comments in bug #507740). [Could someone please verify that?] 2. Paludis does currently allow ebuilds to write to T in pkg_pretend phase, although the PMS says it shouldn't. So, it seems that while no one has replied to you in that 2011 discussion, your words must have been heard. :) 3. My original patch in comment #11 did pass S in src_* phases and substituted it for T in other phases. 4. Robin's concern was that it might break some of the existing stuff, so he suggested limiting it to pkg_{info,nofetch,pretend}. 5. I think we can all agree that the current situation is not looking too good. Therefore, what I suggest is that we apply the patch either from #11 or from #22. This will at least improve the current situation. After that Robin can review if there are any possible fallouts from applying patch in #13 and if none are found, that patch can be also applied, which will eliminate any writes to the file system and we will be in full compilance with the PMS. If we can't get away from writing to the file system in pkg_pretend, then we file a bug or bring up in a new unit of time that the PMS be amended to permit T to be writable, considering that both PMs already allow that in the physical world. Any objections?
(In reply to Ulrich Müller from comment #26) > Let's please fix this a controlled way, not by switching from one workaround > in the eclass to another one. Compared to other distributions fighting portage packet deps can already be hard. But editing ebuilds and removing pretend functions after a sync is where average linux users I know stop believing in Gentoo Portage. Since the PMs are in daily use, a quick fix is justified. Do the proper solution later. Just my 2p... (In reply to Timothy Jones from comment #27) [...] > permit T to be writable, considering that both PMs already allow that in the > physical world. FYI: Not sure what you mean by "already allow that". All I still get with latest paludis/cave is sandbox violations for more than a year now.
I've brought this up in gentoo-pms: http://thread.gmane.org/gmane.linux.gentoo.pms/778
(In reply to F. Kater from comment #28) > (In reply to Timothy Jones from comment #27) > > permit T to be writable, considering that both PMs already allow that in the > > physical world. > > FYI: Not sure what you mean by "already allow that". All I still get with > latest paludis/cave is sandbox violations for more than a year now. Internally paludis/cave does already allow the ebuilds to write to T inside pkg_pretend. The reason you are seeing the violations is that they try to write to /usr/src/linux (or / in the case of nvidia-drivers). What the patch in comment #11 does is it directs them to write to ${T} in phases other than src_*. The patch in comment #13 eliminates the writes altogether.
(In reply to Ulrich Müller from comment #29) > I've brought this up in gentoo-pms: > http://thread.gmane.org/gmane.linux.gentoo.pms/778 Thank you sir. Robin, would you please commit it now?
Bump
*** Bug 475910 has been marked as a duplicate of this bug. ***
If there is a reason to delay the above resolutions please comment here why. If not, please do not delay and fix this issue.
Guys, seriously, what's the holdup? Let's get this thing applied and move on to bigger and better things. There are still plenty of bugs to sit on :)
(In reply to F. Kater from comment #34) > If there is a reason to delay the above resolutions please comment here why. (In reply to Timothy Jones from comment #35) > Guys, seriously, what's the holdup? As outlined in comment 23, comment 24, comment 26 and comment 29; a controlled and right PMS resolution, not a commit that replaces one workaround with another.
Nothing's going on on the PMS mailing list, right? ;) Patch in #13 doesn't require any changes to PMS and resolves the issue completely. The only thing remaining is to verify that it doesn't break ancient kernels.
(In reply to Tom Wijsman (TomWij) from comment #36) > (In reply to F. Kater from comment #34) > > If there is a reason to delay the above resolutions please comment here why. > (In reply to Timothy Jones from comment #35) > > Guys, seriously, what's the holdup? > > As outlined in comment 23, comment 24, comment 26 and comment 29; a > controlled and right PMS resolution, not a commit that replaces one > workaround with another. Tom, what makes you think these are workarounds? These are actual fixes -- not workarounds. My patch in comment #13 prevents kernel's make from writing to the filesystem. Which is what Uli brought up in comment #23. Further, my patch in comment #11 addresses the fact that S only exists in src_* phases, which is what Uli brought up in comment #24. So, if the outcome of the comment #29 happens to be that T should indeed be writable, this patch already addresses it. Again these patches are fixes -- not workarounds. I have provided the patches and explained the logic behind them and I feel like I am being run around by various devs. These patches are *not* workarounds -- they are fixes. They are fixes -- not workarounds.
(In reply to Kirill Elagin from comment #37) > Nothing's going on on the PMS mailing list, right? ;) One voice against, and nobody in support of the proposed change to the spec. So at this point, a solution that doesn't write to the filesystem would be preferred, from PMS point of view.
(In reply to Kirill Elagin from comment #37) > Nothing's going on on the PMS mailing list, right? ;) The PMS takes time to change, not from one day to the other; give it some time. > Patch in #13 doesn't require any changes to PMS and resolves the issue > completely. The only thing remaining is to verify that it doesn't break > ancient kernels. Yes, it should be verified that this does not break anything; entirely removing code that's supposed to be there for a reason is somewhat tricky, unless it turns out that this is deprecated and no longer applicable today. Blaming the history of the eclass can shed some light on why this particular code was added. (In reply to Timothy Jones from comment #38) > Tom, what makes you think these are workarounds? These aren't my thoughts, it refers to comment #26 and others; since earlier comments seem to suggest they weren't read into at all, hence the repetition. > These are actual fixes -- not workarounds. [... cut part ...] Again these patches are fixes -- not workarounds. [... cut part ...] These patches are *not* workarounds -- they are fixes. They are fixes -- not workarounds. Repeating yourself extraneously is not going to cause this to be resolved any sooner than the PMS and/or manpower allows it to. That they are fixes, doesn't imply that they don't work around PMS; so, someone can see some as workarounds. Comment #13 is promising; however, it should undergo testing before applying.
http://sources.gentoo.org/cgi-bin/viewvc.cgi/gentoo-x86/eclass/linux-info.eclass?view=log Relevant and important revisions that I could find in annotation and above link: Revision 1.4: Fixing up support for KBUILD_OUTPUT. Revision 1.78: Bug #300306, #167385: One side effect of the non-exec check version for the kernel makefile is that it did not pick up appends, breaking builds on some patched kernels. Try the exec version first now, with a fallback. Revision 1.81: Bug #286145: More flexible logic for OUTPUT_DIR defaults. Revision 1.82: Merge first part of patch from bug #87242: New warning output method and support for another assignment type in makefile non-exec parsing. Revision 1.83: Factor out the function used to validate the makefile for the extraction method. Revision 1.102: get_version: extract version info with getfilevar_noexec as it should always work, and is much faster than evaluating with make; reported by Doug Anderson from ChromiumOS Especially revision 1.78 seems of interest here, 1.81 seems somewhat relevant as well; wouldn't the removal of the code in question reintroduce the bugs mentioned in those two revisions, or will they remain working for other reasons? The performance improvement from 1.102 might also be another imposed regression.
(In reply to Tom Wijsman (TomWij) from comment #41) > http://sources.gentoo.org/cgi-bin/viewvc.cgi/gentoo-x86/eclass/linux-info. > eclass?view=log I've looked through these several times already. It's easy to get lost in all the changes, but here is a brief rundown: Rev 1.1 introduces getfilevar() that parses the kernel Makefile and is used to extract KV_* and KBUILD_OUTPUT from it. Rev 1.6 changes getfilevar() to use make instead of parsing, because it is "more reliable" and the developer johnm felt it was a "bandaid" to parse the Makefile. Rev 1.61 introduces getfilevar_noexec(), which again parses the Makefile not entirely unlike the original getfilevar did. It also switches everything within the linux-info.eclass to use the shiny new getfilevar_noexec instead of the original getfilevar. But getfilevar is left in place because: "If the variable is defined, you will run into problems. See getfilevar for those cases." These "cases" must be outside of the scope of linux-info.eclass, because at this point there is *nothing* in the linux-info.eclass that uses getfilevar. Long live getfilevar. Rev 1.78 switches back to using getfilevar and only tries getfilevar_noexec, if the former fails. This was based on bug #300306, caused by the linux-omap Makefile, which had the following line in it: # Add custom flags here to avoid conflict with updates EXTRAVERSION := $(EXTRAVERSION)-omap1 which caused getfilevar_noexec to fail. Let me point out 2 things: (1) linux-omap is not a standard package in the portage, so whoever reported the bug #300306 should have been told that it is not supported. (2) As of 2009-09-28 and this commit: https://git.kernel.org/cgit/linux/kernel/git/tmlind/linux-omap.git/commit/?id=f286f075db630e2266ddd011248f4533b55e8f85 point (1) is irrelevant, as is the bug #300306, since linux-omap no longer sets the EXTRAVERSION as above. We can now happily switch back to getfilevar_noexec. SIDE NOTE: This revision also claims to handle bug #167385, but in fact, it doesn't. Look at the last comment in the bug. Also, if you look through the linux-info.eclass at that revision, you will notice that KV_LOCAL is extracted from the name of the kernel dir in KV_OUT_DIR or KV_DIR, or from .config file using getfilevar_noexec function (inside linux_chkconfig_string). It's not using getfilevar at all. Rev 1.102: switches back to using getfilevar_noexec to extract KV_*, but leaves out KBUILD_OUTPUT. So, we are back to square zero, with the exception of KBUILD_OUTPUT. In other words, at this point in time the *only* use of the getfilevar in linux-info.eclass is to extract KBUILD_OUTPUT from the Makefile: if [[ -z ${OUTPUT_DIR} ]]; then # Decide the function used to extract makefile variables. local mkfunc=$(get_makefile_extract_function "${KERNEL_MAKEFILE}") # And if we didn't pass it, we can take a nosey in the Makefile. OUTPUT_DIR=$(${mkfunc} KBUILD_OUTPUT "${KERNEL_MAKEFILE}") fi And if you carefully re-read my comment #12, you will notice that the only time KBUILD_OUTPUT is altered in the Makefile, is when O= is passed to make on the command line. And since O= is not passed to make on the command line when it is called from within getfilevar, there is absolutely no reason to "take a nosey in the Makefile." That is exactly what the patch -- *not* workaround -- in comment #13 does. SIDE NOTE: As a matter of fact, we can also drop getfilevar, since there is nothing else inside linux-info.eclass or anywhere else in the portage that's using it. Long live getfilevar. What does this leave with? This leaves us with the possibility of breaking older Makefiles, which robit was going to check... Right, Robin? Well, today I went back as far as Linux v2.6.32 and looked at its Makefile. And what did I find? I found that the section of the Makefile dealing with KBUILD_OUTPUT is (with the exception of 3 disrelated lines) is *identical* to that snipped in comment #12. So, no the patch in comment #13 will not break any older kernels. Case closed. So now, please apply the patch and let's close this bug.
By the way, I understand completely, why it's so hard to convince you guys to apply these patches. It's because none of you experience these access violations. Only the paludis users do. And the reason you are not seeing these is because of a bug in the portage itself, which allows it to write to any arbitrary location from within pkg_pretend, which is the exact opposite of what the PMS says. And since you guys (Tom and Ulrich) are so insistent that we follow the PMS, would you please take a peek at bug #507740 and help get it resolved? Then all portage users will also start experiencing these violations and we will all be on the same page. Thank you in advance. -TJ
And the procrastination continues...
It's been 10 days now. Why hasn't this been commited yet?
(In reply to Timothy Jones from comment #45) > It's been 10 days now. Why hasn't this been commited yet? +1 wtf?
(In reply to Timothy Jones from comment #44) (In reply to Timothy Jones from comment #45) (In reply to Alex Turbov from comment #46) Can you stop adding noise to this bug please? It won't speed up resolution, only all people in CC will receive an unnecessary e-mail message for every comment.
The minimal patch is added now. I haven't verified the get_makefile_extract_function case because I don't have enough solid time.
The bug has been referenced in the following commit(s): https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=5a3acd443c3347568d14d000776f46177317744a commit 5a3acd443c3347568d14d000776f46177317744a Author: Ulrich Müller <ulm@gentoo.org> AuthorDate: 2020-06-24 16:00:10 +0000 Commit: Ulrich Müller <ulm@gentoo.org> CommitDate: 2020-06-26 20:11:12 +0000 linux-info.eclass: Pass M=${T} to the Linux Makefile unconditionally. Using M="${S}" breaks in the pkg_setup phase where the S variable is not valid. Previous commit messages don't give any rationale why some phases would need the dir pointing to ${S}. Therefore, use ${T} in all phases unconditionally. Closes: https://bugs.gentoo.org/729178 Bug: https://bugs.gentoo.org/469210 Acked-by: Thomas Deutschmann <whissi@gentoo.org> Signed-off-by: Ulrich Müller <ulm@gentoo.org> eclass/linux-info.eclass | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)
I think we can close this now.
