The default bug view has changed. See this FAQ.

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)

(Assignee)

Description

5 years ago
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.
(Assignee)

Comment 1

5 years ago
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+
(Assignee)

Comment 3

5 years ago
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)
(Assignee)

Updated

5 years ago
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+
(Assignee)

Comment 5

5 years ago
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)
(Assignee)

Updated

5 years ago
Attachment #621395 - Attachment is obsolete: true
(Assignee)

Comment 6

5 years ago
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)
(Assignee)

Updated

5 years ago
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+
(Assignee)

Comment 8

5 years ago
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.