FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Gentoo > Gentoo Development

 
 
LinkBack Thread Tools
 
Old 02-24-2010, 04:03 PM
Jeroen Roovers
 
Default 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.
 
Old 02-24-2010, 04:16 PM
"Paweł Hajdan, Jr."
 
Default 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
 
Old 02-25-2010, 02:23 PM
Daniel Black
 
Default 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)
 
Old 03-07-2010, 01:39 AM
Mike Frysinger
 
Default 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
-mike
 
Old 03-08-2010, 03:34 AM
Jeroen Roovers
 
Default 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

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


jer
 
Old 03-08-2010, 03:56 AM
Mike Frysinger
 
Default 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
 

Thread Tools




All times are GMT. The time now is 07:30 AM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org