bootstrap should rebuild LLVM when it changes

[jyn514]

I tried this code:

; ./configure --set llvm.download-ci-llvm=false
; x build --stage 0 rustc_llvm
; (cd src/llvm_project && git apply)
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp b/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
index dd86144d16..43395b1ecd 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
@@ -153,6 +153,8 @@ static void dumpStringOffsetsSection(raw_ostream &OS, DIDumpOptions DumpOpts,
                                      StringRef StringSection,
                                      DWARFContext::unit_iterator_range Units,
                                      bool LittleEndian) {
+  OS << "debug: dumpStringOffsetsSection" << "\n";
+
   auto Contributions = collectContributionData(Units);
   DWARFDataExtractor StrOffsetExt(Obj, StringOffsetsSection, LittleEndian, 0);
   DataExtractor StrData(StringSection, LittleEndian, 0);
; x build --stage 0 rustc_llvm

I expected to see this happen: bootstrap rebuilds LLVM, since it's changed.

Instead, this happened: bootstrap does nothing.

The problem is that we 're using a stamp file and assume that if the commit hasn't changed then none of the files have changed either:

rust/src/bootstrap/llvm.rs

Lines 108 to 123 in 95e8b6a

	let stamp = out_dir.join("llvm-finished-building");
	let stamp = HashStamp::new(stamp, builder.in_tree_llvm_info.sha());
	if stamp.is_done() {
	if stamp.hash.is_none() {
	builder.info(
	"Could not determine the LLVM submodule commit hash. \
	Assuming that an LLVM rebuild is not necessary.",
	);
	builder.info(&format!(
	"To force LLVM to rebuild, remove the file `{}`",
	stamp.path.display()
	));
	}
	return Ok(res);
	}

I am not sure why we do that? Are we assuming that CMake rebuilds too frequently? That seems like something we should fix upstream ...

cc @rust-lang/wg-llvm, I think this would make your lives easier.

Meta

HEAD is branched from ba6f5e3.

[jyn514]

hmm, #64156 looks related. I am not sure why that would be the correct behavior? If CFLAGS has changed we should rebuild; I would expect distros to make sure CFLAGS matches between invocations of bootstrap.

[jyn514]

The stamp checking for the "normal" case, when not building from a source tarball, has been there since the initial commit of bootstrap: 046e687#diff-ddd5cbd996a9fbebe15be7bb2d9254ea4c1336d6c2e47a5da2a76719df572c01R30-R38

[cuviper]

Are you trying to address the git "dirty" case, or the non-git case, or both?

[jyn514]

Both, but primarily the git case since that's how most people are developing I think.

[cuviper]

Ok, I agree the git case is where folks may actually be making changes, and I still think it's ok not to automatically "notice" changes in the non-git case.

LLVM is big enough that checking for dirty git may not be cheap though. On my Linux system with NVME, it takes ~0.5s after dropping page caches, and ~0.15s cached. On systems with slower disks, virus scanners, etc., it may be a lot worse.

[nikic]

Yeah, I don't think this is particularly important. The main issue with making changes to LLVM is #109070, which prevents the change from being picked up even if you do commit it.