Issue 3967 - Properly handle non-existing file-mappings

packager.py

Index: packager.py
diff --git a/packager.py b/packager.py
index 4c6be80fd7b235adf2a98b17182d9eb847b70fbe..365849635fa47d551eda61a082999e5ce61add4a 100644
--- a/packager.py
+++ b/packager.py
@@ -91,19 +91,18 @@ class Files(dict):
         dict.__setitem__(self, key, value)
 
     def isIncluded(self, relpath):
+        return relpath.split('/')[0] in self.includedFiles
+
+    def isignored(self, relpath):

[kzar, 2017/07/17 09:37:40]

Nit: Should it be is_ignored? (Kind of a question for Sebastian / Vasily)

[tlucas, 2017/07/17 12:29:48]

for the sake of readability i would agree. Acknowledged

         parts = relpath.split('/')
-        if not parts[0] in self.includedFiles:
-            return False
-        for part in parts:
-            if part in self.ignoredFiles:
-                return False
-        return True
+        return any([part in self.ignoredFiles for part in parts])
 
     def read(self, path, relpath='', skip=()):
         if os.path.isdir(path):
             for file in os.listdir(path):
                 name = relpath + ('/' if relpath != '' else '') + file
-                if name not in skip and self.isIncluded(name):
+                if (name not in skip
+                        and self.isIncluded(name) and not self.isignored(name)):

[kzar, 2017/07/17 09:37:40]

The indentation looks a bit funky here, but I'm rusty with Python. Did you check
that linting passes?

[tlucas, 2017/07/17 12:29:48]

flake8 didn't nag about this at all - and from what i understood, Sebastian
wanted exactly that (PEP-8 adherence)

[kzar, 2017/07/17 12:32:52]

Fair enough, let's see what he thinks.

[Sebastian Noack, 2017/07/17 14:02:31]

The following two styles are acceptable according to PEP-8
(https://www.python.org/dev/peps/pep-0008/#indentation):

  if (this_is_one_thing and
      that_is_another_thing):
      do_something()

  if (this_is_one_thing
          and that_is_another_thing):
      do_something()

I personally, prefer the former, which is also more consistent with what we have
in our existing code base.

Another option is to use some temporary variables in order to avoid wrapping in
the first place:

  included = self.isIncluded(name) and not self.isignored(name)
  if name not in skip and included:
      ...

[tlucas, 2017/07/17 16:34:12]

As discussed, outsourcing the logic to a variable imho seems to be the most
elegant solution for this, Done

                     self.read(os.path.join(path, file), name, skip)
         else:
             with open(path, 'rb') as file:
@@ -115,9 +114,10 @@ class Files(dict):
         for item in mappings:
             target, source = item
 
-            # Make sure the file is inside an included directory
-            if '/' in target and not self.isIncluded(target):
+            # Adhere to given ignorance

[kzar, 2017/07/17 09:37:40]

I'm not sure this comment adds much, maybe just remove it?

[tlucas, 2017/07/17 12:29:48]

Acknowledged.

+            if '/' in target and self.isignored(target):
                 continue
+
             parts = source.split('/')
             path = os.path.join(os.path.dirname(item.source), *parts)
             if os.path.exists(path):