Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Git integration #4

Open
jrfaller opened this issue Feb 14, 2024 · 9 comments
Open

Git integration #4

jrfaller opened this issue Feb 14, 2024 · 9 comments

Comments

@jrfaller
Copy link
Contributor

Hi there! I am trying to integrate roseau with Git and I have this so far:

[difftool "roseau"]
cmd = roseau.sh "$LOCAL" "$REMOTE"

[alias]
rs = difftool -d -t roseau

I have modified roseau.sh as such:

#!/bin/bash

cd target

if [ "$#" -ne 2 ]; then
    echo "Usage: ./roseau.sh pathV1 pathV2"
    exit 1
fi

java -jar roseau-0.0.2-SNAPSHOT-jar-with-dependencies.jar --diff --v1="$1" --v2="$2"

And amazingly I can run a git rs to see the diff between the current code and HEAD. However the output is not that interesting for instance:

SLF4J: Failed to load class "org.slf4j.impl.StaticLoggerBinder".
SLF4J: Defaulting to no-operation (NOP) logger implementation
SLF4J: See http://www.slf4j.org/codes.html#StaticLoggerBinder for further details.
Parsing: 7332
API extraction: 96
Parsing: 7441
API extraction: 16
API diff: 215

(Note it should be nice to set SLF4J to no-op. to avoid this noisy output at the beginning)

Maybe I am not using the CLI correctly and there is a way to have more info into the console? Or maybe it would require to introduce a plain text formatter?

Anyway I think that would be awesome to be able to use roseau this way!

@jrfaller
Copy link
Contributor Author

jrfaller commented Feb 14, 2024

OK got there with a slight modification of private void diff(Path v1, Path v2, Path report) throws Exception :

(BTW AFAIU there was a bug because the public void breakingChangesReport() throws IOException was NOT using the report path, so I updated it to use the supplied Path report instead of a hardcoded path)

	private void diff(Path v1, Path v2, Path report) throws Exception {
		CompletableFuture<API> futureV1 = CompletableFuture.supplyAsync(() -> buildAPI(v1));
		CompletableFuture<API> futureV2 = CompletableFuture.supplyAsync(() -> buildAPI(v2));

		CompletableFuture.allOf(futureV1, futureV2).join();

		API apiV1 = futureV1.get();
		API apiV2 = futureV2.get();

		// API diff
		Stopwatch sw = Stopwatch.createStarted();
		APIDiff diff = new APIDiff(apiV1, apiV2);
		List<BreakingChange> bcs = diff.diff();
		System.out.println("API diff: " + sw.elapsed().toMillis());

		if (report != null) 
			diff.breakingChangesReport(report);
		else {
			for (BreakingChange bc : bcs)
				System.out.println(bc);
		}
	}

Got the logger to shut-up by adding this to pom.xml

        <dependency>
            <groupId>org.slf4j</groupId>
            <artifactId>slf4j-nop</artifactId>
            <version>2.0.9</version>
        </dependency>
        <dependency>
            <groupId>org.slf4j</groupId>
            <artifactId>slf4j-api</artifactId>
            <version>2.0.9</version>
        </dependency>

And finally got my output!

$ git rs
Parsing: 4269
API extraction: 142
Parsing: 5670
API extraction: 5
API diff: 18
BC[kind=METHOD_REMOVED, symbol=com.github.maracas.roseau.diff.APIDiff.breakingChangesReport]

Funny thing, the parameter I added to breakingChangesReport induces a METHOD_REMOVED BC so I guess this is working 😄

Side note, it would be awesome to get rid of the debug information about parsing time and such, maybe using a logger or maybe using a verbose option?

@tdegueul
Copy link
Contributor

It's a super cool use case; thanks for the idea!

Roseau now only outputs the breaking changes on the standard output, by default, debug information is now hidden behind the --verbose option (dcf060c). Spoon's logger should now shut up with the no-op dependency.

Got your use case to work with the following configuration:

[alias]                                                                                                                                                                                                                                                                                                                                                                                                                 
  rs = difftool -d -t roseau                                                                                                                                                                                                                                                                                                                                                                                            
[difftool "roseau"]                                                                                                                                                                                                                     
  cmd = java -jar [...]/roseau-0.0.2-SNAPSHOT-jar-with-dependencies.jar --diff --v1 "$LOCAL" --v2 "$REMOTE" 

Output:

$ git rs                              
BC[kind=METHOD_REMOVED, symbol=com.github.maracas.roseau.diff.APIDiff.toString]

Don't expect too much accuracy in Roseau's results yet, but that's a neat use case. Will keep support in the future ;)

@jrfaller
Copy link
Contributor Author

That's really nice! Well done.

I was thinking of using it in a blocking way to return an error code when the diff contains breaking change (for libs with 0 BC policies). Apparently, there is a trustExitCode option in Git but I am not able to get it working. I will already try to create a PR with the fail mode.

@tdegueul
Copy link
Contributor

tdegueul commented Mar 2, 2024

When running difftool, Git creates two temporary directories where it only stores the files that have changed, not the whole repository pre- and post-diff.

$LOCAL is set to the name of the temporary file containing the contents of the diff pre-image and $REMOTE is set to the name of the temporary file containing the contents of the diff post-image

That's a pity, because roseau cannot be accurate in this situation: it parses and analyses partial projects where most references cannot be resolved, which hurts the precision of the results (although I'd have to check that in more detail).

@tdegueul
Copy link
Contributor

tdegueul commented Mar 2, 2024

For reference, here's a simple illustration of the issue with a repository containing two interfaces:

IA.java

interface IA {

}

IB.java

public interface IB extends IA {}

The patch is:

interface IA {
-
+ void foo();
}

This is breaking, because the public interface IB has a new abstract method. However, because the file IB.java hasn't changed, the temporary directories created by difftool contain:

├── v1
│   └── IA.java
└── v2
    └── IA.java -> /path/to/repository/IA.java

So roseau cannot see the change on IB and doesn't report it.

@tdegueul
Copy link
Contributor

tdegueul commented Mar 2, 2024

A somewhat convoluted workaround is to create an alias that takes two revisions as inputs and uses git archive to copy the whole trees for these revisions in temporary directories before invoking roseau:

[alias]
  bc = "!f() { \
    V1=$(mktemp -d); \
    V2=$(mktemp -d); \
    echo "Copying $1 to $V1 and $2 to $V2"; \
    git archive $1 | tar -x -C "$V1"; \
    git archive $2 | tar -x -C "$V2"; \
    roseau --verbose --diff --v1 "$V1" --v2 "$V2"; \
  }; f"

This works well when you can provide the two revisions but:

  • It won't let you analyze a git diff or git diff --staged because those don't have revisions assigned yet, which really hurts the usefulness of roseau
  • It won't scale to large repositories

@jrfaller
Copy link
Contributor Author

I guess that if you want to diff with the current version then you just need to use . as V2? Should we put this in the documentation?

@tdegueul
Copy link
Contributor

That could work, but the alias would be quite convoluted and would deviate from the standard git diff syntax.

@jrfaller
Copy link
Contributor Author

I agree that your alias is IMHO not in the scope of what an alias should be. For instance, it is not portable to Windows, etc... Another possibility is to use something like JGit and implement the integration on Roseau's side. WDYT?

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

No branches or pull requests

2 participants