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

RESOLVED FIXED in mozilla15

Status

()

Core
Build Config
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: glandium, Assigned: glandium)

Tracking

Trunk
mozilla15
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

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.
Created attachment 621019 [details] [diff] [review]
Avoid Preprocessor.py applying filters to file names given on the command line
Attachment #621019 - Flags: review?(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.
Attachment #621019 - Flags: review?(ted.mielczarek) → review+
Created attachment 621395 [details] [diff] [review]
Avoid Preprocessor.py applying filters to file names given on the command line

Adds plenty of tests
Attachment #621395 - Flags: review?(ted.mielczarek)
Attachment #621019 - Attachment is obsolete: true
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. :-(
Attachment #621395 - Flags: review?(ted.mielczarek) → review+
Created attachment 621502 [details] [diff] [review]
Avoid Preprocessor.py applying filters to file names given on the command line

I think this covers the assertRaises as context manager cases.
Attachment #621502 - Flags: review?(ted.mielczarek)
Attachment #621395 - Attachment is obsolete: true
Created attachment 621524 [details] [diff] [review]
Avoid Preprocessor.py applying filters to file names given on the command line

Man, python 2.5 is a PITA.
Attachment #621524 - Flags: review?(ted.mielczarek)
Attachment #621502 - Attachment is obsolete: true
Attachment #621502 - Flags: review?(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)
Attachment #621524 - Flags: review?(ted.mielczarek) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/b65d61a1d0ee
Target Milestone: --- → mozilla15
http://hg.mozilla.org/mozilla-central/rev/b65d61a1d0ee
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.