Skip to content

Switch patch application from java-diff-utils to JGit - #1

Open
Laurettta wants to merge 1 commit into
mainfrom
feature/switch-diffutils-to-jgit
Open

Laurettta wants to merge 1 commit into
mainfrom
feature/switch-diffutils-to-jgit

Conversation

@Laurettta

Copy link
Copy Markdown

Switched java-diff-utils for JGit's ApplyCommand, the old library couldn't create new files from a patch, only edit existing ones. Tested and both cases work now.

@Laurettta
Laurettta requested a review from adamretter September 8, 2026 16:00
@marmoure

Copy link
Copy Markdown

This was tested as part of evolvedbinary/elemental#233 , I can confirm the whole workflow is working, but I didn't dig deep into the workings of the plugin.

@Laurettta
Laurettta force-pushed the feature/switch-diffutils-to-jgit branch from 5a6e150 to 07ea7e6 Compare September 11, 2026 15:42

@adamretter adamretter left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some small changes and questions please

@@ -0,0 +1,50 @@
<!--

Copyright [2024] [Lukas Mansour]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this is a new file created by us. It should not have:

Copyright [2024] [Lukas Mansour]
Modifications: Copyright (C) 2026, Evolved Binary Ltd

It should only have:

Copyright (C) 2026, Evolved Binary Ltd

You'll need to make some config changes to the license-maven-plugin to support that.

import static org.junit.jupiter.api.Assertions.fail


/*

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same for this license declaration

}

final String content = new String(Files.readAllBytes(patchedFile))
assertTrue(content.contains("Hello, Elemental!"), "Expected content was not found in: " + patchedFile)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would content.equals be more accurate here?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this file?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's an empty placeholder, without it, src/main/resources would be empty, which means Git won't track the folder, which then makes the patch step fail because its target directory doesn't exist yet.

Comment thread src/it/apply-patch-new-file/pom.xml Outdated
<!--

Copyright [2024] [Lukas Mansour]
Modifications: Copyright (C) 2026, Evolved Binary Ltd

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue with the license declaration as above.



/*
* Copyright [2024] [Lukas Mansour]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue with the license declaration as above.

}

final String content = new String(Files.readAllBytes(createdFile))
assertTrue(content.contains("brand new file"), "Expected content was not found in: " + createdFile)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would content.equals be more accurate here?

@Laurettta
Laurettta force-pushed the feature/switch-diffutils-to-jgit branch from 07ea7e6 to 6f40643 Compare September 21, 2026 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants