Bazel: improved lazy lfs files

[redsun82]

This reintroduces lazy lfs file rules that were removed in #16117, now improved.

The new rules will make the actual file download go through bazel's download manager, which includes:

• caching into the repository cache
• sane limiting of concurrent downloads
• retries

The bulk of the work is done by git_lfs_probe.py, which will use the LFS protocolq to output short lived download URLs that can be consumed by repository_ctx.download.

[redsun82]

This is shown to work on this internal PR

[criemen]

One major question, I'll have a more detailed look at the rest of this later, but I don't think there's gonna be other big questions coming.

[criemen]

Some more comments - what I don't understand yet is if we now still replace the LFS pointers on the source file system with the instantiated files.
As far as I understood, the previous iteration of the LFS rules did exactly that, therefore easily allowing developers to inspect/replace the lazy LFS files that were part of the build process. Is that still the case now?

[redsun82]

Some more comments - what I don't understand yet is if we now still replace the LFS pointers on the source file system with the instantiated files. As far as I understood, the previous iteration of the LFS rules did exactly that, therefore easily allowing developers to inspect/replace the lazy LFS files that were part of the build process. Is that still the case now?

Yes, and better than before. Previously, the non-LFS pointer files would still go through a smudge filter, which meant the files would be copied. Now:

• git_lfs_probe.py will detect a non-LFS pointer file and output local for those
• the bazel rule counterparts will interpret that to
  • symlink the file in case of lfs_files rules, or
  • call repository_ctx.extract directly on the file in case of lfs_archive ones.

[criemen]

Ah, but that's the other way, no?

I meant:

• The user hasn't downloaded the LFS file yet, so on-disk in the source tree it's a LFS pointer
• The user builds with bazel, bazel acquires the file from LFS
• After the build, is the file in the source tree still a LFS pointer, or has it been replaced with the real file?

I thought in the previous iteration of these rules, pointers were replaced by actual files, whereas now, that doesn't seem to be the case (anymore?).

[redsun82]

Ah, but that's the other way, no?

I meant:

• The user hasn't downloaded the LFS file yet, so on-disk in the source tree it's a LFS pointer
• The user builds with bazel, bazel acquires the file from LFS
• After the build, is the file in the source tree still a LFS pointer, or has it been replaced with the real file?

I thought in the previous iteration of these rules, pointers were replaced by actual files, whereas now, that doesn't seem to be the case (anymore?).

Ah, sorry, I didn't understand initially. That mode of operation (replacing pointer files) was one temporary iteration of how I had implemented it at some point, but it didn't stick 🙂. The final version currently present on semmle-code doesn't do that (see here). Generally changing the source tree during the build should be avoided

[criemen]

LGTM, thanks for the in-depth discussion!