[PATCH] D19872: Produce cpio files for --reproduce

[Rui Ueyama via llvm-commits]

ruiu added a comment.

Very interesting. The choice I thought after "ar" was tar, but cpio seems to be indeed simpler than tar. I knew that the file format exists but I didn't know the details about it.

I'd probably define Cpio class, but this is fine as well.


================
Comment at: ELF/DriverUtils.cpp:130
@@ +129,3 @@
+  OS << "000000";                        // c_dev
+  OS << "000000";                        // c_ino // FIXME: should be unique
+  OS << "100664";                        // c_mode: C_ISREG | rw-rw-r--
----------------
If you are not going to fix it, it is not a fixme. Please change the comment to say that this should be unique, but since no one cares, we don't bother to set a unique number.

================
Comment at: ELF/DriverUtils.cpp:145-146
@@ -122,4 +144,4 @@
+void elf::maybeCopyInputFile(StringRef Src, StringRef Data) {
   std::string Dest = getDestPath(Src);
-  StringRef Dir = path::parent_path(Dest);
-  if (std::error_code EC = fs::create_directories(Dir)) {
-    error(EC, Dir + ": can't create directory");
+  maybePrintCpioMember(Dest, Data);
+}
----------------
When storing pathnames to an archive, you don't want to prepend Config->Reproduce string, no? For example, if you are storing /home/ruiu/foo.obj, then you store the file as that path, instead of /home/ruiu/repro/home/ruiu/foo.obj.

================
Comment at: ELF/DriverUtils.cpp:202
@@ +201,3 @@
+  SmallString<128> Dest;
+  path::append(Dest, Config->Reproduce, "response.txt");
+  maybePrintCpioMember(Dest, Data);
----------------
Ditto. You want to store this as just "response.txt" instead of a full path.

================
Comment at: ELF/Error.cpp:32
@@ -31,2 +31,3 @@
   HasError = true;
+  maybeCloseReproArchive();
 }
----------------
You want to do this for fatal() as well.

================
Comment at: test/ELF/reproduce-linkerscript.s:9
@@ -8,2 +8,3 @@
 # RUN: ld.lld build/foo.script -o bar --reproduce repro
+# RUN: cpio -id < repro.cpio
 # RUN: diff build/foo.script repro/%:t.dir/build/foo.script
----------------
I'm not sure if cpio is available everywhere. If not, you need to define "cpio" and "REQUIRES: cpio".


http://reviews.llvm.org/D19872