[gentoo-dev] Proposed change to savedconfig.eclass

[gentoo-dev] Proposed change to savedconfig.eclass

[Jeroen Roovers]

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

    Hello developers,


this has annoyed me for a long time.

restore_config() dies when it cannot find a saved config file, while
later on in any ebuild that uses savedconfig.eclass, it will save the
config file anyhow. That means it will not use an edited saved config
file during the first emerge, which is to be expected and should not be
fatal.

The current forces you to emerge the same ebuild twice using _different_
USE flags to achieve anything:

1. USE="-savedconfig" emerge cat/foo
2. $EDITOR /etc/portage/savedconfig/cat/foo
3. USE="savedconfig" emerge cat/foo

This is quite illogical.

With the patch applied it should be enough to set USE=savedconfig
globally, run `emerge foo', edit the saved config file(s) and run
`emerge foo' again, without having to change the USE flag:

0. euse -E savedconfig # Set USE=savedconfig globally
1. emerge cat/foo
2. $EDITOR /etc/portage/savedconfig/cat/foo
3. emerge cat/foo
4. Profit!

The attached patch actually accomplishes two things:

1) It removes some old code[1] in an elif that apparently never gets
executed (or we would have seen bash syntax bugs filed loooong ago).
2) It makes restore_config non-fatal on the first emerge with
USE=savedconfig.

If no one objects, I will look forward to committing the patch in a
week or two.


Regards,
     jer


[1] Already present in the first commit.

[-- Attachment #2: savedconfig.eclass-donotdie.patch --]
[-- Type: text/x-patch, Size: 1248 bytes --]

Index: savedconfig.eclass
===================================================================
RCS file: /var/cvsroot/gentoo-x86/eclass/savedconfig.eclass,v
retrieving revision 1.12
diff -u -B -r1.12 savedconfig.eclass
--- savedconfig.eclass	30 Oct 2009 16:46:41 -0000	1.12
+++ savedconfig.eclass	24 Feb 2010 16:52:07 -0000
@@ -111,16 +111,14 @@
 		pushd "${found}" > /dev/null
 		treecopy . "${dest}" || die "Failed to restore ${found} to $1"
 		popd > /dev/null
-	elif [[ -a {found} ]]; then
-		die "do not know how to handle non-file/directory ${found}"
 	else
 		# maybe the user is screwing around with perms they shouldnt #289168
 		if [[ ! -r ${base} ]] ; then
 			eerror "Unable to read ${base} -- perms are screwed ?"
 			die "fix your system"
 		fi
-		eerror "No saved config to restore - please remove USE=savedconfig or"
-		eerror "provide a configuration file in ${PORTAGE_CONFIGROOT}/etc/portage/savedconfig/${CATEGORY}/${PN}"
-		die "config file needed when USE=savedconfig is specified"
+		ewarn "No saved config to restore - please remove USE=savedconfig or"
+		ewarn "provide a configuration file in ${PORTAGE_CONFIGROOT}/etc/portage/savedconfig/${CATEGORY}/${PN}"
+		ewarn "Your config file(s) will not be used this time"
 	fi
 }

Re: [gentoo-dev] Proposed change to savedconfig.eclass

["Paweł Hajdan, Jr."]

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

While you're touching this, could you improve this part a bit:

# maybe the user is screwing around with perms they shouldnt #289168
if [[ ! -r ${base} ]] ; then
 eerror "Unable to read ${base} -- perms are screwed ?"
 die "fix your system"
fi

I understand frustration caused by weird things people are doing with
systems, but sometimes it can be even caused by some tool's error or
whatever. IMHO these are not good error messages. I'd prefer something
like this:

# Make sure we don't hit a problem with permissions, bug #289168
if [[ ! -r ${base} ]] ; then
 eerror "Unable to read ${base}. Please run chmod 755 ${base}"
 eerror "and try again."
 die "unable to read ${base}"
fi

Thanks,
Paweł Hajdan jr


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

Re: [gentoo-dev] Proposed change to savedconfig.eclass

[Daniel Black]

On Thursday 25 February 2010 04:03:16 Jeroen Roovers wrote:
>     Hello developers,
> 
> this has annoyed me for a long time.
> 
> restore_config() dies when it cannot find a saved config file, while
> later on in any ebuild that uses savedconfig.eclass, it will save the
> config file anyhow. That means it will not use an edited saved config
> file during the first emerge, which is to be expected and should not be
> fatal.
> 
> The current forces you to emerge the same ebuild twice using _different_
> USE flags to achieve anything:
> 
> 1. USE="-savedconfig" emerge cat/foo
> 2. $EDITOR /etc/portage/savedconfig/cat/foo
> 3. USE="savedconfig" emerge cat/foo
> 
> This is quite illogical.


Fixing this is fine my me.

Daniel
(illogical savedconfig.eclass author)

Re: [gentoo-dev] Proposed change to savedconfig.eclass

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 174 bytes --]

On Wednesday 24 February 2010 12:03:16 Jeroen Roovers wrote:
> If no one objects, I will look forward to committing the patch in a
> week or two.

commit it already :p
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [gentoo-dev] Proposed change to savedconfig.eclass

[Jeroen Roovers]

On Sat, 6 Mar 2010 21:39:32 -0500
Mike Frysinger <vapier@gentoo.org> wrote:

> On Wednesday 24 February 2010 12:03:16 Jeroen Roovers wrote:
> > If no one objects, I will look forward to committing the patch in a
> > week or two.
> 
> commit it already :p

Thanks for the reminder. In the same commit have also made the remaining
eerror/die messages nicer, since no one objected.


     jer

Re: [gentoo-dev] Proposed change to savedconfig.eclass

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 1107 bytes --]

On Wednesday 24 February 2010 12:16:24 Paweł Hajdan, Jr. wrote:
> While you're touching this, could you improve this part a bit:
> 
> # maybe the user is screwing around with perms they shouldnt #289168
> if [[ ! -r ${base} ]] ; then
>  eerror "Unable to read ${base} -- perms are screwed ?"
>  die "fix your system"
> fi
> 
> I understand frustration caused by weird things people are doing with
> systems, but sometimes it can be even caused by some tool's error or
> whatever. IMHO these are not good error messages. I'd prefer something
> like this:
> 
> # Make sure we don't hit a problem with permissions, bug #289168
> if [[ ! -r ${base} ]] ; then
>  eerror "Unable to read ${base}. Please run chmod 755 ${base}"
>  eerror "and try again."
>  die "unable to read ${base}"
> fi

the issue is in basic assumptions.  you're assuming that -r means a chmod will 
fix it because the error is due to missing +r bits.  i make no assumptions and 
merely propose the most likely problem category (missing +r bits).  a subtle, 
but important, distinction (at least in my mind).
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]