Skip to content

Track additional Git object information#6347

Merged
shanman190 merged 3 commits intomainfrom
feat/git-object-information
Dec 4, 2025
Merged

Track additional Git object information#6347
shanman190 merged 3 commits intomainfrom
feat/git-object-information

Conversation

@shanman190
Copy link
Copy Markdown
Contributor

@shanman190 shanman190 commented Nov 26, 2025 •
edited
Loading

What's changed?

Create a new marker to track additional Git object information, presently scoped to Object ID.

Alternate name: GitObjectId.

What's your motivation?

For Git binary patch support, we have a need to go from Quark to Remote. In order to generate the index line, this requires the old object id and the new object id. For other LST elements, we generate the object id on the fly by generating the sha1 of the raw printed bytes. For Quark, this isn't possible and needs a different solution.

Anything in particular you'd like reviewers to focus on?

N/A

Anyone you would like to review specifically?

Anybody

Have you considered any alternatives or workarounds?

We could have added the object id as a field on Quark or SourceFile, but it was preferred to utilize a dedicated marker instead.

Any additional context

Another long standing issue is related to file mode. The current implementation of FileAttributes is very lossy with respect to this information. A possibility would be to capture the FileMode#getBits() from JGit since it would cost O(1) to retrieve and add to this marker. Doing so, I'd probably suggest that we change the name of the marker to something else, such as GitTreeEntry to be more accurate of it's purpose.

Additionally, on Windows, Files#isExecutable(Path) always returns true. So this can manifest in the Git patch later as a file being marked as executable when it normally wouldn't be due to this JVM quirk.

Checklist

  • I've added unit tests to cover both positive and negative cases
  • I've read and applied the recipe conventions and best practices
  • I've used the IntelliJ IDEA auto-formatter on affected files

@shanman190 shanman190 self-assigned this Nov 26, 2025
@shanman190 shanman190 added the enhancement New feature or request label Nov 26, 2025
@github-project-automation github-project-automation Bot moved this to In Progress in OpenRewrite Nov 26, 2025
This was referenced Nov 26, 2025
Merged
Merged
@shanman190 shanman190 force-pushed the feat/git-object-information branch from ae4ad2d to bc26461 Compare December 2, 2025 18:27
@shanman190 shanman190 marked this pull request as ready for review December 2, 2025 18:45
shanman190
shanman190 commented Dec 2, 2025
View reviewed changes
Comment thread rewrite-core/src/main/java/org/openrewrite/marker/FileMode.java Outdated
@shanman190 shanman190 force-pushed the feat/git-object-information branch from bc26461 to 8c1049b Compare December 2, 2025 18:59
@shanman190
Copy link
Copy Markdown
Contributor Author

shanman190 commented Dec 3, 2025

These JS tests are known to be flaky, but are unrelated to the changes here.

FAIL test/javascript/add-import.test.ts (122.281 s)
  ● AddImport visitor › object imports (non-function references) › should add import for vitest vi object when referenced

    thrown: "Exceeded timeout of 30000 ms for a test.

    Add a timeout value to this test to increase the timeout, if this is a long-running test. See https://jestjs.io/docs/api#testname-fn-timeout."

      1351 |
      1352 |     describe('object imports (non-function references)', () => {
    > 1353 |         test('should add import for vitest vi object when referenced', async () => {
           |             ^
      1354 |             const spec = new RecipeSpec();
      1355 |             spec.recipe = fromVisitor(createRemoveThenAddImportVisitor("vitest", "vi"));
      1356 |

      at test/javascript/add-import.test.ts:1353:13
      at test/javascript/add-import.test.ts:1352:13
      at Object.<anonymous> (test/javascript/add-import.test.ts:163:9)

FAIL test/javascript/node-resolution-result.test.ts (96.013 s)
  ● NodeResolutionResult marker › should attach NodeResolutionResult marker to package.json

    thrown: "Exceeded timeout of 30000 ms for a test.
    Add a timeout value to this test to increase the timeout, if this is a long-running test. See https://jestjs.io/docs/api#testname-fn-timeout."

      33 | describe("NodeResolutionResult marker", () => {
      34 |
    > 35 |     test("should attach NodeResolutionResult marker to package.json", async () => {
         |     ^
      36 |         const spec = new RecipeSpec();
      37 |         await withDir(async (repo) => {
      38 |             await spec.rewriteRun(

      at test/javascript/node-resolution-result.test.ts:35:5
      at Object.<anonymous> (test/javascript/node-resolution-result.test.ts:33:9)

@shanman190
Copy link
Copy Markdown
Contributor Author

shanman190 commented Dec 4, 2025 •
edited
Loading

Interestingly enough, apparently Git only supports a couple of the posix file modes:

  • Regular non-executable file (100644)
  • Regular executable file (100755)
  • Symbolic link (120000)
  • Directory (040000)
  • Gitlink/submodule (160000)

All other variations are reduced to their closest match.

This makes the file mode portion a little less useful, but does situate us slightly better with respect to being able to understand the difference between a regular file and a symlink. It also could be the potential foundation needed to model submodules.

@shanman190 shanman190 merged commit a36d6d6 into main Dec 4, 2025
1 of 2 checks passed
@shanman190 shanman190 deleted the feat/git-object-information branch December 4, 2025 16:23
@github-project-automation github-project-automation Bot moved this from In Progress to Done in OpenRewrite Dec 4, 2025
@baflQA baflQA mentioned this pull request Feb 8, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jkschneider jkschneider jkschneider left review comments

Assignees

@shanman190 shanman190

Labels

enhancement New feature or request

Projects

OpenRewrite
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@shanman190 @jkschneider