Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 456076 - portage declares variables like D and ED readonly, making it impossible to localize them
Summary: portage declares variables like D and ED readonly, making it impossible to lo...
Status: CONFIRMED
Alias: None
Product: Portage Development
Classification: Unclassified
Component: Core - Ebuild Support (show other bugs)
Hardware: All Linux
: Normal normal (vote)
Assignee: Portage team
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-07 19:22 UTC by Michał Górny
Modified: 2015-04-26 21:22 UTC (History)
1 user (show)

See Also:
Package list:
Runtime testing required: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2013-02-07 19:22:36 UTC
Portage does mark a number of ebuild variables like D and ED readonly using the bash 'declare -r' command.

Bash concept of readonly variables is utterly stupid and totally unsuited for this. Most importantly, once marked readonly, one can't even declare a local variable with the same name. This also makes it impossible for eclasses to override the value of D for a local scope execution.

AFAICS PMS doesn't prohibit ebuilds from locally overriding the value of those variables. I don't see a reason why it should.
Comment 1 Ciaran McCreesh 2013-02-07 19:24:05 UTC
You can unset them for external processes...
Comment 2 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2013-02-07 19:35:20 UTC
(In reply to comment #1)
> You can unset them for external processes...

No, you can't.

$ DUPA=11 true
bash: DUPA: readonly variable
Comment 3 Ciaran McCreesh 2013-02-07 19:38:59 UTC
env -u.
Comment 4 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2013-02-07 19:42:22 UTC
(In reply to comment #3)
> env -u.

That's not unsetting them for a process but in a process, and that doesn't serve my purpose at all. I want to temporarily override the value in ebuild scope.
Comment 5 Ciaran McCreesh 2013-02-07 19:44:22 UTC
And we don't want you doing that. D and ED can be used internally by package mangler helpers, and bad things happen if you think you're allowed to muck around with them.
Comment 6 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2013-02-07 19:49:58 UTC
(In reply to comment #5)
> And we don't want you doing that. D and ED can be used internally by package
> mangler helpers, and bad things happen if you think you're allowed to muck
> around with them.

And they should be respected by package mangler helpers. Unless you're saying that you simply suck so much at package manager design that you lock everyone in small cells just to make sure they can't do anything useful with them.
Comment 7 Ciaran McCreesh 2013-02-07 19:56:27 UTC
Ah, you've reverted to "you suck". I guess you aren't interested in technical discussion, so there's no point taking this further.
Comment 8 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2013-02-07 19:58:25 UTC
So far, this is not a bug to you and PMS doesn't prohibit me from modifying D.
Comment 9 Zac Medico gentoo-dev 2013-02-07 19:59:52 UTC
(In reply to comment #6)
> And they should be respected by package mangler helpers. Unless you're
> saying that you simply suck so much at package manager design that you lock
> everyone in small cells just to make sure they can't do anything useful with
> them.

If anything, it sounds like an EAPI extension. So, create a proposal that lays out your goals, and post it on the -dev ml.
Comment 10 Ciaran McCreesh 2013-02-07 20:05:30 UTC
(In reply to comment #8)
> So far, this is not a bug to you and PMS doesn't prohibit me from modifying
> D.

Uh, yes it does. "Ebuilds must not attempt to modify any of these variables, unless otherwise specified."
Comment 11 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2013-02-07 20:12:08 UTC
(In reply to comment #10)
> (In reply to comment #8)
> > So far, this is not a bug to you and PMS doesn't prohibit me from modifying
> > D.
> 
> Uh, yes it does. "Ebuilds must not attempt to modify any of these variables,
> unless otherwise specified."

Declaring a local variable with same name != modifying it.
Comment 12 Ciaran McCreesh 2013-02-07 20:14:35 UTC
You were talking about ebuild helpers, not local variables with the same name.
Comment 13 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2013-02-07 20:16:45 UTC
(In reply to comment #12)
> You were talking about ebuild helpers, not local variables with the same
> name.

You started the topic of ebuild helpers.
Comment 14 Ciaran McCreesh 2013-02-07 20:19:31 UTC
Well why do you want a local variable named D or ED? You can just use another name, and avoid confusion for anyone reading your code.
Comment 15 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2013-02-07 20:40:32 UTC
(In reply to comment #14)
> Well why do you want a local variable named D or ED? You can just use
> another name, and avoid confusion for anyone reading your code.

For eclass functions which properly recognize and use D and ED.
Comment 16 Ciaran McCreesh 2013-02-07 20:45:31 UTC
(In reply to comment #15)
> For eclass functions which properly recognize and use D and ED.

...and which don't spawn processes, and all kinds of other things you're neglecting to mention, all of which mean that an apparently trivial change can break things in weird and subtle ways.
Comment 17 SpanKY gentoo-dev 2013-02-08 05:45:24 UTC
marking them read-only (like many other built in variables) has helped prevent people from accidentally overriding them, or trying to dumb stuff that breaks things.  unless you can provide real examples that are clearly hindered by this behavior, then there's no reason to change it.
Comment 18 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2013-02-08 09:46:41 UTC
(In reply to comment #17)
> marking them read-only (like many other built in variables) has helped
> prevent people from accidentally overriding them, or trying to dumb stuff
> that breaks things.  unless you can provide real examples that are clearly
> hindered by this behavior, then there's no reason to change it.

The idea is to use a fake D during parallel python_install() phases to prevent race conditions when people work on same-named files without having to make them explicitly aware of the fake root.
Comment 19 SpanKY gentoo-dev 2013-02-08 23:54:02 UTC
(In reply to comment #18)

seems to me that you could easily accomplish the thing by passing the temp dir as a proper arg to the func rather than abusing the env
Comment 20 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2013-02-09 00:00:45 UTC
(In reply to comment #19)
> (In reply to comment #18)
> 
> seems to me that you could easily accomplish the thing by passing the temp
> dir as a proper arg to the func rather than abusing the env

Then user will have to be aware of a new variable he needs to use when working with installed files before they are moved to real ${D}. And of course, all the existing ebuilds will be broken.
Comment 21 Ulrich Müller gentoo-dev 2013-02-09 09:13:39 UTC
(In reply to comment #20)
> > seems to me that you could easily accomplish the thing by passing the temp
> > dir as a proper arg to the func rather than abusing the env
> 
> Then user will have to be aware of a new variable he needs to use when
> working with installed files before they are moved to real ${D}. And of
> course, all the existing ebuilds will be broken.

IMHO, this would be still the lesser of two evils. It is confusing if ebuilds tamper with the standard variables, so we should leave the spec alone.
Comment 22 SpanKY gentoo-dev 2013-04-03 16:22:39 UTC
(In reply to comment #21)

i don't think the spec requires these variables be marked read-only ?

it is possible to have portage default to read-only variables but allow people to explicitly override those in functions.  the bash behavior is summarized as:
 - if a variable is declared read-only in global scope, it may never be changed
 - if a variable is declared local/read-only in func scope, funcs may override

so instead of doing `declare -r` in global scope, portage would have to add a trampoline func or a main func.

the former would probably be easier to insert into the current code base:
 readonly_vars() {
   declare -r <all vars to localize + make readonly>
   "$@"
 }
but it'd also be more error prone (making sure all the right places go through the trampoline).

the latter would be more work up front (only because the portage code base is generally a mess due to historical origins), but would result in cleaner code and then always work.  basically, disallow mixing func decls & code execution and put all the init work into a main().  then at the end of the file do:
main "$@"; exit
that way in the main() func where you declare `readonly -r D=...`, it's now done in function scope and anyone can override it if they really want.

personally, i would prefer if it were made explicit.  with the current bash, you can do this as easily as just:
  local D
i think it'd be a lot better if you had to do:
  declare +r D=...
i'll make that suggestion upstream.  it won't help now, but who knows in 5 years.

as to the original request, i don't mind it.  i am a little bit concerned someone might mess up and pick the wrong variable (since it would now be as easy as `local D`), but i don't have a feeling how likely this is in practice.  probably not really.  we also don't have to make it so *all* variables are allowed this dynamic scoping, just a select subset (such as D & ED).  in the main func, you can do `declare -gr` to get global read only scoping behavior.