[PATCH] D19872: Produce cpio files for --reproduce
Rafael EspĂndola via llvm-commits
llvm-commits at lists.llvm.org
Tue May 3 09:08:29 PDT 2016
> ================
> 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.
Done
>
> ================
> 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.
It should be repro/home/ruiu/foo.obj. That way extracting the archive
only creates one directory and you can fakeroot into it.
> ================
> 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.
This is just repro/response.txt.
> ================
> Comment at: ELF/Error.cpp:32
> @@ -31,2 +31,3 @@
> HasError = true;
> + maybeCloseReproArchive();
> }
> ----------------
> You want to do this for fatal() as well.
Done
> ================
> 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".
It is included in the gnuwin32 package that we require for testing on
windows, so I think we are good.
I will upload a new patch in one sec.
Cheers,
Rafael
More information about the llvm-commits
mailing list