751865 - Preprocessor.py shouldn't apply filters to file names given on the command line

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Mike Hommey [:glandium]]

With the change in bug 508942, all uses of do_include lead to filters being applied on the file name. But then, if you Preprocessor.py -fsubstitution -Dfoo=bar @foo@.in, Preprocessor.py tries to open bar.in instead of @foo@.in. It is a corner case, but I'm actually using Preprocessor.py this way for ease of Debian packaging.

Comment 1

[Mike Hommey [:glandium]]

Attachment: Avoid Preprocessor.py applying filters to file names given on the command line

Comment 2

[(not currently active) Ted Mielczarek]

Comment on attachment 621019 [details] [diff] [review]
Avoid Preprocessor.py applying filters to file names given on the command line

Review of attachment 621019 [details] [diff] [review]:
-----------------------------------------------------------------

This could use a unit test, but looks fine otherwise.

Comment 3

[Mike Hommey [:glandium]]

Attachment: Avoid Preprocessor.py applying filters to file names given on the command line

Adds plenty of tests

Comment 4

[(not currently active) Ted Mielczarek]

Comment on attachment 621395 [details] [diff] [review]
Avoid Preprocessor.py applying filters to file names given on the command line

Review of attachment 621395 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, just one issue.

::: config/tests/unit-Preprocessor.py
@@ +22,5 @@
> +    f = open('foo', 'r')
> +
> +  will thus assign the NamedIO instance for the file 'foo' to f.
> +  """
> +  def __init__(self, files):

You could write this as **files so you can just pass them as separate args and not an actual list.

@@ +529,5 @@
> +  def test_undefined_variable(self):
> +    f = NamedIO("undefined_variable.in", """#filter substitution
> +@foo@
> +""")
> +    with self.assertRaises(Preprocessor.Error) as cm:

assertRaises as a context manager is unfortunately a Python 2.7 feature. :-(

Comment 5

[Mike Hommey [:glandium]]

Attachment: Avoid Preprocessor.py applying filters to file names given on the command line

I think this covers the assertRaises as context manager cases.

Comment 6

[Mike Hommey [:glandium]]

Attachment: Avoid Preprocessor.py applying filters to file names given on the command line

Man, python 2.5 is a PITA.

Comment 7

[(not currently active) Ted Mielczarek]

Comment on attachment 621524 [details] [diff] [review]
Avoid Preprocessor.py applying filters to file names given on the command line

Review of attachment 621524 [details] [diff] [review]:
-----------------------------------------------------------------

::: config/tests/unit-Preprocessor.py
@@ +560,5 @@
> +      self.pp.do_include(f)
> +    except Preprocessor.Error, exception:
> +      self.assertEqual(exception.key, 'FILE_NOT_FOUND')
> +    else:
> +      self.assertEqual(1, 0)

You can still use assertRaises, it's just sort of annoying. You'd have to write this like:
self.assertRaises(Preprocessor.Error, self.pp.do_include, f)

Comment 8

[Mike Hommey [:glandium]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b65d61a1d0ee

Comment 9

[Ryan VanderMeulen [:RyanVM]]

http://hg.mozilla.org/mozilla-central/rev/b65d61a1d0ee