Add basic include_runfiles to pkg_files. (#724)

* Add basic include_runfiles to pkg_files.

Show it working in a mappings tests.
Improve the mapping test to print something a little more useful.

Next step: Make the same code callable from pkg_tar and pkg_zip

* utf8 wierdness with python vesrions

9 files changed, 146 insertions, 45 deletions

diff --git a/pkg/mappings.bzl b/pkg/mappings.bzl
index b42dd6b..0901b92 100644
--- a/pkg/mappings.bzl
+++ b/pkg/mappings.bzl
@@ -145,8 +145,8 @@ def _do_strip_prefix(path, to_strip, src_file):
 
     if path_norm.startswith(to_strip_norm):
         return path_norm[len(to_strip_norm):]
-    elif src_file.is_directory and (path_norm + '/') == to_strip_norm:
-        return ''
+    elif src_file.is_directory and (path_norm + "/") == to_strip_norm:
+        return ""
     else:
         # Avoid user surprise by failing if prefix stripping doesn't work as
         # expected.
@@ -219,7 +219,13 @@ def _pkg_files_impl(ctx):
     # The input sources are already known.  Let's calculate the destinations...
 
     # Exclude excludes
-    srcs = [f for f in ctx.files.srcs if f not in ctx.files.excludes]
+    srcs = []  # srcs is source File objects, not Targets
+    file_to_target = {}
+    for src in ctx.attr.srcs:
+        for f in src[DefaultInfo].files.to_list():
+            if f not in ctx.files.excludes:
+                srcs.append(f)
+                file_to_target[f] = src
 
     if ctx.attr.strip_prefix == _PKGFILEGROUP_STRIP_ALL:
         src_dest_paths_map = {src: paths.join(ctx.attr.prefix, src.basename) for src in srcs}
@@ -283,6 +289,25 @@ def _pkg_files_impl(ctx):
         else:
             src_dest_paths_map[src_file] = paths.join(ctx.attr.prefix, rename_dest)
 
+    # At this point, we have a fully valid src -> dest mapping for all the
+    # explicitly named targets in srcs. Now we can fill in their runfiles.
+    if ctx.attr.include_runfiles:
+        for src in srcs:
+            target = file_to_target[src]
+            runfiles = target[DefaultInfo].default_runfiles
+            if runfiles:
+                base_path = src_dest_paths_map[src] + ".runfiles"
+                for rf in runfiles.files.to_list():
+                    dest_path = paths.join(base_path, rf.short_path)
+
+                    # print("Add runfile:", rf.path, 'as', dest_path)
+                    have_it = src_dest_paths_map.get(rf)
+                    if have_it:
+                        if have_it != dest_path:
+                            print("same source mapped to different locations", rf, have_it, dest_path)
+                    else:
+                        src_dest_paths_map[rf] = dest_path
+
     # At this point, we have a fully valid src -> dest mapping in src_dest_paths_map.
     #
     # Construct the inverse of this mapping to pass to the output providers, and
@@ -426,6 +451,14 @@ pkg_files = rule(
             default = {},
             allow_files = True,
         ),
+        "include_runfiles": attr.bool(
+            doc = """Add runfiles for all srcs.
+
+            The runfiles are in the paths that Bazel uses. For example, for the
+            target `//my_prog:foo`, we would see files under paths like
+            `foo.runfiles/<repo name>/my_prog/<file>`
+            """,
+        ),
     },
     provides = [PackageFilesInfo],
 )
@@ -560,30 +593,29 @@ pkg_mklink_impl = rule(
     provides = [PackageSymlinkInfo],
 )
 
-def pkg_mklink(name, link_name, target, attributes=None, src=None, **kwargs):
-  """Create a symlink.
-
-  Args:
-    name: target name
-    target: target path that the link should point to.
-    link_name: the path in the package that should point to the target.
-    attributes: file attributes.
-  """
-  if src:
-    if target:
-      fail("You can not specify both target and src.")
-    # buildifier: disable=print
-    print("Warning: pkg_mklink.src is deprecated. Use target.")
-    target = src
-  pkg_mklink_impl(
-      name = name,
-      target = target,
-      link_name = link_name,
-      attributes = attributes,
-      **kwargs,
-  )
-
+def pkg_mklink(name, link_name, target, attributes = None, src = None, **kwargs):
+    """Create a symlink.
 
+    Args:
+      name: target name
+      target: target path that the link should point to.
+      link_name: the path in the package that should point to the target.
+      attributes: file attributes.
+    """
+    if src:
+        if target:
+            fail("You can not specify both target and src.")
+
+        # buildifier: disable=print
+        print("Warning: pkg_mklink.src is deprecated. Use target.")
+        target = src
+    pkg_mklink_impl(
+        name = name,
+        target = target,
+        link_name = link_name,
+        attributes = attributes,
+        **kwargs
+    )
 
 def _pkg_filegroup_impl(ctx):
     files = []
diff --git a/tests/BUILD b/tests/BUILD
index 21b6396..7fa40a5 100644
--- a/tests/BUILD
+++ b/tests/BUILD
@@ -99,10 +99,26 @@ py_test(
     srcs_version = "PY3",
 )
 
+cc_library(
+    name = "liba",
+    srcs = ["a.cc"],
+    data = ["testdata/hello.txt"],
+)
+
+cc_library(
+    name = "libb",
+    srcs = ["b.cc"],
+    data = ["testdata/hello.txt"],
+)
+
 cc_binary(
     name = "an_executable",
     srcs = ["foo.cc"],
     data = ["BUILD"],
+    deps = [
+        ":liba",
+        ":libb",
+    ],
 )
 
 py_test(
diff --git a/tests/a.cc b/tests/a.cc
new file mode 100644
index 0000000..d46545e
--- /dev/null
+++ b/tests/a.cc
@@ -0,0 +1 @@
+int a = 1;
diff --git a/tests/b.cc b/tests/b.cc
new file mode 100644
index 0000000..b20302a
--- /dev/null
+++ b/tests/b.cc
@@ -0,0 +1 @@
+int b = 2;
diff --git a/tests/foo.cc b/tests/foo.cc
index 4359267..3274cf8 100644
--- a/tests/foo.cc
+++ b/tests/foo.cc
@@ -1 +1,20 @@
-int main(int argc, char* argv[]) { return 0; }
+#include <fstream>
+#include <iostream>
+#include <string>
+
+extern int a, b;
+
+// A very roundabout hello world.
+int main(int argc, char* argv[]) {
+  std::string runfiles(argv[0]);
+  runfiles.append(".runfiles");
+  std::string hello(runfiles + "/rules_pkg/tests/testdata/hello.txt");
+  std::fstream fs;
+  fs.open(hello, std::iostream::in);
+  char tmp[1000];
+  fs.read(tmp, sizeof(tmp));
+  fs.close();
+  std::cout << tmp;
+
+  return (a + b > 0) ? 0 : 1;
+}
diff --git a/tests/mappings/BUILD b/tests/mappings/BUILD
index 44752cb..404b25e 100644
--- a/tests/mappings/BUILD
+++ b/tests/mappings/BUILD
@@ -129,6 +129,7 @@ manifest_golden_test(
 write_content_manifest(
     name = "executable_manifest",
     srcs = [
+        "mappings_test.bzl",
         "//tests:an_executable",
     ],
     include_runfiles = True,
diff --git a/tests/mappings/executable.manifest.golden b/tests/mappings/executable.manifest.golden
index b869a1d..ee86633 100644
--- a/tests/mappings/executable.manifest.golden
+++ b/tests/mappings/executable.manifest.golden
@@ -1,5 +1,7 @@
 [
-    {"type": "file", "dest": "an_executable.runfiles/tests/BUILD", "src": "tests/BUILD", "mode": "", "user": null, "group": null, "uid": null, "gid": null, "origin": "@//tests:an_executable"},
-    {"type": "file", "dest": "an_executable.runfiles/tests/an_executable", "src": "tests/an_executable", "mode": "0755", "user": null, "group": null, "uid": null, "gid": null, "origin": "@//tests:an_executable"},
-    {"type": "file", "dest": "an_executable", "src": "tests/an_executable", "mode": "0755", "user": null, "group": null, "uid": null, "gid": null, "origin": "@//tests:an_executable"}
+{"dest":"an_executable.runfiles/tests/BUILD","gid":null,"group":null,"mode":"","origin":"@//tests:an_executable","src":"tests/BUILD","type":"file","uid":null,"user":null},
+{"dest":"an_executable.runfiles/tests/an_executable","gid":null,"group":null,"mode":"0755","origin":"@//tests:an_executable","src":"tests/an_executable","type":"file","uid":null,"user":null},
+{"dest":"an_executable.runfiles/tests/testdata/hello.txt","gid":null,"group":null,"mode":"","origin":"@//tests:an_executable","src":"tests/testdata/hello.txt","type":"file","uid":null,"user":null},
+{"dest":"an_executable","gid":null,"group":null,"mode":"0755","origin":"@//tests:an_executable","src":"tests/an_executable","type":"file","uid":null,"user":null},
+{"dest":"mappings_test.bzl","gid":null,"group":null,"mode":"","origin":"@//tests/mappings:mappings_test.bzl","src":"tests/mappings/mappings_test.bzl","type":"file","uid":null,"user":null}
 ]
diff --git a/tests/mappings/executable.manifest.windows.golden b/tests/mappings/executable.manifest.windows.golden
index b6a22d4..d853c0d 100644
--- a/tests/mappings/executable.manifest.windows.golden
+++ b/tests/mappings/executable.manifest.windows.golden
@@ -1,5 +1,7 @@
 [
-    {"type": "file", "dest": "an_executable.exe.runfiles/tests/BUILD", "src": "tests/BUILD", "mode": "", "user": null, "group": null, "uid": null, "gid": null, "origin": "@//tests:an_executable"},
-    {"type": "file", "dest": "an_executable.exe.runfiles/tests/an_executable.exe", "src": "tests/an_executable.exe", "mode": "0755", "user": null, "group": null, "uid": null, "gid": null, "origin": "@//tests:an_executable"},
-    {"type": "file", "dest": "an_executable.exe", "src": "tests/an_executable.exe", "mode": "0755", "user": null, "group": null, "uid": null, "gid": null, "origin": "@//tests:an_executable"}
+{"dest":"an_executable.exe.runfiles/tests/BUILD","gid":null,"group":null,"mode":"","origin":"@//tests:an_executable","src":"tests/BUILD","type":"file","uid":null,"user":null},
+{"dest":"an_executable.exe.runfiles/tests/an_executable.exe","gid":null,"group":null,"mode":"0755","origin":"@//tests:an_executable","src":"tests/an_executable.exe","type":"file","uid":null,"user":null},
+{"dest":"an_executable.exe.runfiles/tests/testdata/hello.txt","gid":null,"group":null,"mode":"","origin":"@//tests:an_executable","src":"tests/testdata/hello.txt","type":"file","uid":null,"user":null},
+{"dest":"an_executable.exe","gid":null,"group":null,"mode":"0755","origin":"@//tests:an_executable","src":"tests/an_executable.exe","type":"file","uid":null,"user":null},
+{"dest":"mappings_test.bzl","gid":null,"group":null,"mode":"","origin":"@//tests/mappings:mappings_test.bzl","src":"tests/mappings/mappings_test.bzl","type":"file","uid":null,"user":null}
 ]
diff --git a/tests/mappings/manifest_test_lib.py b/tests/mappings/manifest_test_lib.py
index 0534ebe..f403304 100644
--- a/tests/mappings/manifest_test_lib.py
+++ b/tests/mappings/manifest_test_lib.py
@@ -23,19 +23,46 @@ class ContentManifestTest(unittest.TestCase):
 
   run_files = runfiles.Create()
 
-  def assertManifestsMatch(self, expected, got):
+  def assertManifestsMatch(self, expected_path, got_path):
     """Check two manifest files for equality.
 
     Args:
-        expected: The path to the content we expect.
-        got: The path to the content we got.
+        expected_path: The path to the content we expect.
+        got_path: The path to the content we got.
     """
-    e_file = ContentManifestTest.run_files.Rlocation('rules_pkg/' + expected)
-    with open(e_file, mode='rb') as e_fp:
-      expected = json.load(e_fp)
-    expected_dict = {x["dest"]: x for x in expected}
-    g_file = ContentManifestTest.run_files.Rlocation('rules_pkg/' + got)
-    with open(g_file, mode='rb') as g_fp:
-      got = json.load(g_fp)
-    got_dict = {x["dest"]: x for x in got}
-    self.assertEqual(expected_dict, got_dict)
+    e_file = ContentManifestTest.run_files.Rlocation('rules_pkg/' + expected_path)
+    with open(e_file, mode='rt', encoding='utf-8') as e_fp:
+      expected = json.loads(e_fp.read())
+    expected_dict = {x['dest']: x for x in expected}
+    g_file = ContentManifestTest.run_files.Rlocation('rules_pkg/' + got_path)
+    with open(g_file, mode='rt', encoding='utf-8') as g_fp:
+      got = json.loads(g_fp.read())
+    got_dict = {x['dest']: x for x in got}
+    # self.assertEqual(expected_dict, got_dict)
+
+    ok = True
+    expected_dests = set(expected_dict.keys())
+    got_dests = set(got_dict.keys())
+    for dest, what in expected_dict.items():
+      got = got_dict.get(dest)
+      if got:
+        self.assertDictEqual(what, got)
+      else:
+        print('Missing expected path "%s" in manifest' % dest)
+        ok = False
+    for dest, what in got_dict.items():
+      expected = expected_dict.get(dest)
+      if expected:
+        self.assertDictEqual(expected, what)
+      else:
+        print('Got unexpected path "%s" in manifest:' % dest, what)
+        ok = False
+
+    if not ok:
+      print('To update the golden file:')
+      print('  cp bazel-bin/%s %s' % (got_path, expected_path))
+      print('or')
+      print('============= snip ==========')
+      print(got_dict.values())
+      print('============= snip ==========')
+    self.assertTrue(ok)