Linux Archive

Linux Archive (http://www.linux-archive.org/)
-   Gentoo Development (http://www.linux-archive.org/gentoo-development/)
-   -   Proposed change to savedconfig.eclass (http://www.linux-archive.org/gentoo-development/331278-proposed-change-savedconfig-eclass.html)

Jeroen Roovers 02-24-2010 04:03 PM

Proposed change to savedconfig.eclass
 
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.

"Paweł Hajdan, Jr." 02-24-2010 04:16 PM

Proposed change to savedconfig.eclass
 
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

Daniel Black 02-25-2010 02:23 PM

Proposed change to savedconfig.eclass
 
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)

Mike Frysinger 03-07-2010 01:39 AM

Proposed change to savedconfig.eclass
 
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

Jeroen Roovers 03-08-2010 03:34 AM

Proposed change to savedconfig.eclass
 
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

Mike Frysinger 03-08-2010 03:56 AM

Proposed change to savedconfig.eclass
 
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


All times are GMT. The time now is 01:33 AM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.