[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