1260066 - Don't allow to use sandbox primitives from anywhere but global scope and templates

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Comment 1

[Mike Hommey [:glandium]]

Attachment: MozReview Request: Bug 1260066 - Move add_old_configure_arg to the global scope. r?nalexander

We've done this for set_config, set_define, imply_option, and
add_old_configure_assignment. Do it for add_old_configure_arg as well.

Review commit: https://reviewboard.mozilla.org/r/42697/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/42697/

Comment 2

[Mike Hommey [:glandium]]

Attachment: MozReview Request: Bug 1260066 - Move last uses of check_prog that weren't in the global scope to the global scope. r?nalexander

Review commit: https://reviewboard.mozilla.org/r/42699/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/42699/

Comment 3

[Mike Hommey [:glandium]]

Attachment: MozReview Request: Bug 1260066 - Don't allow to use sandbox primitives from anywhere but global scope and templates. r?nalexander

The initial goal of templates was to provide a way to write shorter
constructs for some generic tasks during configure. But the limitations
of the sandbox and the properties of templates made them used for more
general functions.

Consequently, this led to templates having to be available from
anywhere, which, in turn, led to difficult to introspect constructs.
With bug 1257823, we've made almost everything use set_config and
similar functions from the global scope, but we don't enforce that
those primitives are only used at the global scope.

This change does that: it enforces that primitives are only used at
the global scope. Or in templates.

Now, since templates were used for other purposes than generic uses
of other primitives, we now allow non-template functions to be declared.
Those can be used everywhere, but don't have access to the sandbox
primitives.

Review commit: https://reviewboard.mozilla.org/r/42701/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/42701/

Comment 4

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8735320 [details]
MozReview Request: Bug 1260066 - Move add_old_configure_arg to the global scope. r?nalexander

https://reviewboard.mozilla.org/r/42697/#review39341

Comment 5

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8735321 [details]
MozReview Request: Bug 1260066 - Move last uses of check_prog that weren't in the global scope to the global scope. r?nalexander

https://reviewboard.mozilla.org/r/42699/#review39343

::: moz.configure:159
(Diff revision 1)
> -@depends(target)
> +        )
> -def linux_programs(target):
>      if target.os == 'GNU' and target.kernel == 'Linux':
> -        check_prog('RPMBUILD', ('rpmbuild',), allow_missing=True)
> +        return namespace(RPMBUILD=('rpmbuild',))
> +
> +check_prog('DSYMUTIL', delayed_getattr(extra_programs, 'DSYMUTIL'),

How can this succeed?  Suppose we're not on Darwin, so extra_programs returns a namespace not including 'DSYMUTIL'.  Then the `delayed_getattr` returns `None`, and `check_prog` should fail: https://dxr.mozilla.org/mozilla-central/source/build/moz.configure/checks.configure#58.

What am I missing?

Comment 6

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8735322 [details]
MozReview Request: Bug 1260066 - Don't allow to use sandbox primitives from anywhere but global scope and templates. r?nalexander

https://reviewboard.mozilla.org/r/42701/#review39347

This looks okay to me, but help me understand the scope of declared functions.  Suppose `a.configure` defines `f` and `b.configure` includes `a.configure`.  Is `f` available in `b.configure`?  I assume yes (there's an `exec_file` in there)?

Comment 7

[Mike Hommey [:glandium]]

(In reply to Nick Alexander :nalexander from comment #5)
> How can this succeed?  Suppose we're not on Darwin, so extra_programs
> returns a namespace not including 'DSYMUTIL'.  Then the `delayed_getattr`
> returns `None`, and `check_prog` should fail:
> https://dxr.mozilla.org/mozilla-central/source/build/moz.configure/checks.
> configure#58.
> 
> What am I missing?

bug 1259960

Comment 8

[Mike Hommey [:glandium]]

(In reply to Nick Alexander :nalexander from comment #6)
> Comment on attachment 8735322 [details]
> MozReview Request: Bug 1260066 - Don't allow to use sandbox primitives from
> anywhere but global scope and templates. r?nalexander
> 
> https://reviewboard.mozilla.org/r/42701/#review39347
> 
> This looks okay to me, but help me understand the scope of declared
> functions.  Suppose `a.configure` defines `f` and `b.configure` includes
> `a.configure`.  Is `f` available in `b.configure`?  I assume yes (there's an
> `exec_file` in there)?

Same as templates and depends, yes.

Comment 9

[Mike Hommey [:glandium]]

Comment on attachment 8735321 [details]
MozReview Request: Bug 1260066 - Move last uses of check_prog that weren't in the global scope to the global scope. r?nalexander

See comment 7

Comment 10

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8735321 [details]
MozReview Request: Bug 1260066 - Move last uses of check_prog that weren't in the global scope to the global scope. r?nalexander

https://reviewboard.mozilla.org/r/42699/#review39389

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/26a63e35fb25
https://hg.mozilla.org/integration/mozilla-inbound/rev/7c1b33d35a5b
https://hg.mozilla.org/integration/mozilla-inbound/rev/63338edce3ba

Comment 12

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/36e9b1eebcf2

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/26a63e35fb25
https://hg.mozilla.org/mozilla-central/rev/7c1b33d35a5b
https://hg.mozilla.org/mozilla-central/rev/63338edce3ba
https://hg.mozilla.org/mozilla-central/rev/36e9b1eebcf2