Remove built cache of previous git commits.

[spoojaryTT]

Ticket

Link to Github Issue

Problem description

Previously built files might still be reused when the tt_metal is updated.

What's changed

1. During build, in cmake, add compile defines for GIT_COMMIT_HASH. Use the VERSION_HASH set by parent CMakeLists.txt
2. Build.cpp would check for the git hash define (GIT_COMMIT_HASH), if hash is not found, proceed without changing any previously built folders and root folder will be 'built/'
3. If git hash is found, the root folder will be 'built/GIT_COMMIT_HASH'.
4. If git hash is found, and TT_METAL_SKIP_DELETING_BUILT_CACHE is NOT set, delete all directories in 'built' except for 'built/GIT_COMMIT_HASH' folder
5. If git hash is found, and TT_METAL_SKIP_DELETING_BUILT_CACHE is set, no directory is deleted. Root for current build will be 'built/GIT_COMMIT_HASH'

Checklist

• Post commit CI passes - https://github.com/tenstorrent/tt-metal/actions/runs/11966119330
• Blackhole Post commit (if applicable)
• Model regression CI testing passes (if applicable)
• Device performance regression CI testing passes (if applicable)
• New/Existing tests provide coverage for changes

[blozano-tt]

Looks good overall. I have only one gripe.

[afuller-TT]

I'm good with this. The CMake changes look good (once we fix the name of git).

I'm not familiar with the JIT build and haven't combed through the code to know why we're loading stale items. This approach feels like a hammer to me, but again I don't know the nuances so you know what's needed here.

[blozano-tt]

I'm good with this. The CMake changes look good (once we fix the name of git).

I'm not familiar with the JIT build and haven't combed through the code to know why we're loading stale items. This approach feels like a hammer to me, but again I don't know the nuances so you know what's needed here.

I also find it weird.

I asked previously why we can't just put things in /tmp/ and let them get deleted after the process completes.

[tt-rkim]

We can't really test this change in CI without doing another build since it's a compile define

[spoojaryTT]

We can't really test this change in CI without doing another build since it's a compile define

If we want to avoid compile define, we can move finding of git hash to inside metal code. Use system() and pipe the output to a file. With this, we can probably test that root path is as expected.
Thoughts @tt-rkim @pgkeller

[tt-rkim]

Please do not add runtime code to get source control information. That'll break pipelines, anyway.

I'm more concerned of how you're gonna write an automated test for this at all.

Also, once @afuller-TT 's changes from here https://github.com/tenstorrent/tt-metal/pull/15400/files go in, we can change your CMake changes to parse data from VERSION_HASH so it's all in one place.

Can we address @blozano-tt and @afuller-TT 's comments about this being a big hammer? I know we've thought about this before. Are there no smaller hammers we can use to invalidate the cache of previous commits?

[pgkeller]

Please do not add runtime code to get source control information. That'll break pipelines, anyway.

I'm more concerned of how you're gonna write an automated test for this at all.

Also, once @afuller-TT 's changes from here https://github.com/tenstorrent/tt-metal/pull/15400/files go in, we can change your CMake changes to parse data from VERSION_HASH so it's all in one place.

Can we address @blozano-tt and @afuller-TT 's comments about this being a big hammer? I know we've thought about this before. Are there no smaller hammers we can use to invalidate the cache of previous commits?

discussion prior to the PR (on devlopers channel, I think) people were worried about leaving stale files behind, hence the delete. the way I see it:

1. We need a key in the path. how we get the key I don't care, sounds like we need to wait for @afuller-TT's change
2. Optionally we can clean up. Sounds like developers want that and you don't. We can just delete that

does this work?

[tt-rkim]

We can do 2 for now until @afuller-TT 's change hits, then we should be able to do 1 and easily provide what the hash here while accounting for some release (git archive) developer flows.

How's that sound, or am I misunderstanding? @afuller-TT @pgkeller

[tt-rkim]

Build looks good, but I still don't see a test to test out the other build path where VERSION_HASH is not defined? Am I blind?

[spoojaryTT]

Build looks good, but I still don't see a test to test out the other build path where VERSION_HASH is not defined? Am I blind?

Yes, we wanted to keep the solution simple, and decided to take the approach of defines. Unfortunately that does make it hard to add automated test for this.

[tt-rkim]

Sure, then we can leave it as a dev feature with no testing.

Approved~!