[gentoo-portage-dev] PATCH: ebuild unpack sources ${T}/environment during setup phase (bug 85803)

[gentoo-portage-dev] PATCH: ebuild unpack sources ${T}/environment during setup phase (bug 85803)

[Zac Medico]

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

http://bugs.gentoo.org/show_bug.cgi?id=85803

This trivial patch seems to work for me but I'm not sure if it will cause regressions or not.  Feedback is appreciated.

Zac

[-- Attachment #2: portage-2.0.51.22-unpack-env.patch --]
[-- Type: text/x-patch, Size: 569 bytes --]

Index: portage-2.0.51.22/bin/ebuild.sh
===================================================================
--- portage-2.0.51.22.orig/bin/ebuild.sh
+++ portage-2.0.51.22/bin/ebuild.sh
@@ -11,7 +11,7 @@ if [ ! -z "${PORTAGE_GPG_DIR}" ]; then
 	SANDBOX_PREDICT="${SANDBOX_PREDICT}:${PORTAGE_GPG_DIR}"
 fi
 
-if [ "$*" != "depend" ] && [ "$*" != "clean" ] && [ "$*" != "nofetch" ]; then
+if [ "$*" != "depend" ] && [ "$*" != "clean" ] && [ "$*" != "nofetch" ] && [ "$*" != "setup" ]; then
 	if [ -f "${T}/environment" ]; then
 		source "${T}/environment" &>/dev/null
 	fi

Re: [gentoo-portage-dev] PATCH: ebuild unpack sources ${T}/environment during setup phase (bug 85803)

[Brian Harring]

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

On Wed, Sep 07, 2005 at 07:20:44PM -0700, Zac Medico wrote:
> http://bugs.gentoo.org/show_bug.cgi?id=85803
> 
> This trivial patch seems to work for me but I'm not sure if it will cause 
> regressions or not.  Feedback is appreciated.
Questionable in relation to binpkgs; binpkg phase execution is rather 
haphazard/broken anyways, but...

question is if the build env (unpack -> install phases) needs to bleed 
through to the > install phases; if so, for binpkgs this change will 
break that.

'protecting' the ebuild env state when setup is executed would address 
this bug; basically, filtering what's saved/reloaded for the env 
dumps.

Dunno.  On the fence on this one- I went the route of just locking 
down chunks of the supplied ebuild*sh env, such that they were bound 
to the portage installation, not the ebuild's generated env; think 
that's the best long term solution, but it's not something that can be 
deployed in stable yet.
~harring

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [gentoo-portage-dev] PATCH: ebuild unpack sources ${T}/environment during setup phase (bug 85803)

[Brian Harring]

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

On Wed, Sep 07, 2005 at 07:20:44PM -0700, Zac Medico wrote:
> http://bugs.gentoo.org/show_bug.cgi?id=85803
> 
> This trivial patch seems to work for me but I'm not sure if it will cause 
> regressions or not.  Feedback is appreciated.
> 
> Zac

> Index: portage-2.0.51.22/bin/ebuild.sh
> ===================================================================
> --- portage-2.0.51.22.orig/bin/ebuild.sh
> +++ portage-2.0.51.22/bin/ebuild.sh
> @@ -11,7 +11,7 @@ if [ ! -z "${PORTAGE_GPG_DIR}" ]; then
>  	SANDBOX_PREDICT="${SANDBOX_PREDICT}:${PORTAGE_GPG_DIR}"
>  fi
>  
> -if [ "$*" != "depend" ] && [ "$*" != "clean" ] && [ "$*" != "nofetch" ]; then
> +if [ "$*" != "depend" ] && [ "$*" != "clean" ] && [ "$*" != "nofetch" ] && [ "$*" != "setup" ]; then
>  	if [ -f "${T}/environment" ]; then
>  		source "${T}/environment" &>/dev/null
>  	fi

Any further thoughts on this?
I'd like to see something done about the bug, but the binpkg setup 
issue stands still.
~harring

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [gentoo-portage-dev] PATCH: ebuild unpack sources ${T}/environment during setup phase (bug 85803)

[Zac Medico]

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

Brian Harring wrote:
> Any further thoughts on this?
> I'd like to see something done about the bug, but the binpkg setup 
> issue stands still.
> ~harring

We can avoid the problem of the crufty ${T}/environment if we run "ebuild.sh clean" before each package in the merge list.  This approach should be much less likely to cause a regression.  It is compatible with FEATURES=keepwork because keepwork is implemented inside ebuild.sh itself.

Zac

[-- Attachment #2: pre-merge-clean.patch --]
[-- Type: text/x-patch, Size: 951 bytes --]

Index: portage/bin/emerge
===================================================================
--- portage.orig/bin/emerge
+++ portage/bin/emerge
@@ -1849,6 +1849,15 @@ class depgraph:
 					and x[0] != "blocks" \
 					and mysysdict.has_key(portage.cpv_getkey(x[2])) \
 					and not ("--buildpkg" in myopts)
+					
+			# Clean up ${T} before merge (see bugs 85803 and 105706).
+			short_msg = "emerge: ("+str(mergecount)+" of "+str(len(mymergelist))+") "+x[pkgindex]+" Clean Pre"
+			emergelog(" === ("+str(mergecount)+" of "+str(len(mymergelist))+") Pre-Merge Cleaning ("+x[pkgindex]+"::"+y+")", short_msg=short_msg)
+			retval=portage.doebuild(y,"clean",myroot,self.pkgsettings,edebug,cleanup=1)
+			if (retval == None):
+				portage_util.writemsg("Unable to run required binary.\n")
+				sys.exit(127)
+					
 			if x[0] in ["ebuild","blocks"]:
 				if (x[0]=="blocks") and ("--fetchonly" not in myopts):
 					raise Exception, "Merging a blocker"

[gentoo-portage-dev] PATCH: clean PORTAGE_TMPDIR (bugs 85803 and 105706)

[Zac Medico]

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

http://bugs.gentoo.org/show_bug.cgi?id=85803
http://bugs.gentoo.org/show_bug.cgi?id=105706

There are a few cases where unwanted directories are left behind in PORTAGE_TMPDIR with varying amounts of files inside.  When packages are merged this may occur with the --buildpkgonly option or with binary packages.  It may also occur when a package ia unmerged.  The attached patch should clean up all of these cases.  Feedback is appreciated.

Zac

[-- Attachment #2: clean_builddir.patch --]
[-- Type: text/x-patch, Size: 1310 bytes --]

Index: bin/emerge
===================================================================
--- bin/emerge	(revision 2199)
+++ bin/emerge	(working copy)
@@ -2007,6 +2007,7 @@
 
 			# Unsafe for parallel merges
 			del portage.mtimedb["resume"]["mergelist"][0]
+			portage.clean_builddir(x[pkgindex].split("/")[1])
 
 		emergelog(" *** Finished. Cleaning up...")
 
Index: pym/portage.py
===================================================================
--- pym/portage.py	(revision 2199)
+++ pym/portage.py	(working copy)
@@ -6425,6 +6425,7 @@
 				writemsg("!!! FAILED postrm: "+str(a)+"\n")
 				sys.exit(123)
 
+		clean_builddir(self.pkg)
 		self.unlockdb()
 
 	def isowner(self,filename,destroot):
@@ -7025,6 +7026,14 @@
 		"Is this a regular package (does it have a CATEGORY file?  A dblink can be virtual *and* regular)"
 		return os.path.exists(self.dbdir+"/CATEGORY")
 
+def clean_builddir(mypkg):
+	for feature in ("noclean","keepwork","keeptemp"):
+		if feature in features:
+			return
+	builddir=os.path.join(settings["PORTAGE_TMPDIR"],"portage",mypkg)
+	if os.path.exists(builddir):
+		shutil.rmtree(builddir)
+
 def cleanup_pkgmerge(mypkg,origdir):
 	shutil.rmtree(settings["PORTAGE_TMPDIR"]+"/portage-pkg/"+mypkg)
 	if os.path.exists(settings["PORTAGE_TMPDIR"]+"/portage/"+mypkg+"/temp/environment"):

Re: [gentoo-portage-dev] PATCH: clean PORTAGE_TMPDIR (bugs 85803 and 105706)

[Brian Harring]

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

On Sun, Oct 30, 2005 at 04:33:28AM -0800, Zac Medico wrote:
> http://bugs.gentoo.org/show_bug.cgi?id=85803
> http://bugs.gentoo.org/show_bug.cgi?id=105706
> 
> There are a few cases where unwanted directories are left behind in 
> PORTAGE_TMPDIR with varying amounts of files inside.  When packages are 
> merged this may occur with the --buildpkgonly option or with binary 
> packages.  It may also occur when a package ia unmerged.  The attached 
> patch should clean up all of these cases.  Feedback is appreciated.

Correct the BUILDDIR misses in doebuild imo; this seems a bit like a 
bandaid over the fact doebuild's clean only cleanses ${T} .

tag the shutil.rmtree into doebuild instead of adding a secondary 
function that must be used, is my opinion- tracing out cleanup and 
mydo == "clean" demonstrates where the miss is.

~harring

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [gentoo-portage-dev] PATCH: clean PORTAGE_TMPDIR (bugs 85803 and 105706)

[Zac Medico]

Brian Harring wrote:
> On Sun, Oct 30, 2005 at 04:33:28AM -0800, Zac Medico wrote:
> 
>>http://bugs.gentoo.org/show_bug.cgi?id=85803
>>http://bugs.gentoo.org/show_bug.cgi?id=105706
>>
>>There are a few cases where unwanted directories are left behind in 
>>PORTAGE_TMPDIR with varying amounts of files inside.  When packages are 
>>merged this may occur with the --buildpkgonly option or with binary 
>>packages.  It may also occur when a package ia unmerged.  The attached 
>>patch should clean up all of these cases.  Feedback is appreciated.
> 
> 
> Correct the BUILDDIR misses in doebuild imo; this seems a bit like a 
> bandaid over the fact doebuild's clean only cleanses ${T} .
> 
> tag the shutil.rmtree into doebuild instead of adding a secondary 
> function that must be used, is my opinion- tracing out cleanup and 
> mydo == "clean" demonstrates where the miss is.
> 
> ~harring


Indeed, metaphorically speaking, my patch _is_ a bandaid.  However, stable portage has many bandaids.  While this certainly in not the best long term solution, it seems to work quite well in the short term. ;)

I choose the bandaid method in order to keep the patch as small as possible and still keep PORTAGE_TMPDIR clean.  AFAICT it would take a *much* larger patch to clean up all the same cases without the bandaid approach.

On a side note, my patch should be changed so that it doesn't try to clean_builddir when things like "--pretend" and "--fetchonly" are in emerge's myopts.

Zac
-- 
gentoo-portage-dev@gentoo.org mailing list

Re: [gentoo-portage-dev] PATCH: clean PORTAGE_TMPDIR (bugs 85803 and 105706)

[Jason Stubbs]

On Monday 31 October 2005 05:29, Zac Medico wrote:
> I choose the bandaid method in order to keep the patch as small as possible
> and still keep PORTAGE_TMPDIR clean.  AFAICT it would take a *much* larger
> patch to clean up all the same cases without the bandaid approach.
>
> On a side note, my patch should be changed so that it doesn't try to
> clean_builddir when things like "--pretend" and "--fetchonly" are in
> emerge's myopts.

The exceptions are likely not the only ones. The exceptions to the exceptions 
to the ... is what is bad about bandaids. I don't know what the "proper" fix 
is in this case, but if you try it you'll probably find it's smaller and/or 
less work that you guessed it to be.

--
Jason Stubbs
-- 
gentoo-portage-dev@gentoo.org mailing list

Re: [gentoo-portage-dev] PATCH: clean PORTAGE_TMPDIR (bugs 85803 and 105706)

[Zac Medico]

Jason Stubbs wrote:
> On Monday 31 October 2005 05:29, Zac Medico wrote:
> 
>>I choose the bandaid method in order to keep the patch as small as possible
>>and still keep PORTAGE_TMPDIR clean.  AFAICT it would take a *much* larger
>>patch to clean up all the same cases without the bandaid approach.
>>
>>On a side note, my patch should be changed so that it doesn't try to
>>clean_builddir when things like "--pretend" and "--fetchonly" are in
>>emerge's myopts.
> 
> 
> The exceptions are likely not the only ones. The exceptions to the exceptions 
> to the ... is what is bad about bandaids. I don't know what the "proper" fix 
> is in this case, but if you try it you'll probably find it's smaller and/or 
> less work that you guessed it to be.
> 

Yeah, I agree with you and Brian.  I don't expect a proper fix to be difficult or time consuming (though I expect the patch size to be a bit larger due to minor refactoring that seems necessary). Anyways, I wanted to see what a small bandaid patch would look like so that it could be compared in size and complexity to a more proper fix.

Zac
-- 
gentoo-portage-dev@gentoo.org mailing list