[PATCH] D19872: Produce cpio files for --reproduce
Rui Ueyama via llvm-commits
llvm-commits at lists.llvm.org
Tue May 3 08:50:21 PDT 2016
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
More information about the llvm-commits
mailing list