-
Notifications
You must be signed in to change notification settings - Fork 7
Release 2.3.0 #157
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
Merged
Release 2.3.0 #157
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is useful when trying to do `mvn deploy:deploy-file` or similar. Also sometimes the wrong `pom.xml` inside of the jar is chosen making the deployment invalid.
This is useful to allow reuse of DiffDetective for further builds using nix.
This is necessary for overriding dependencies of DiffDetective.
Enhance the nix setup
The interface `PropositionalFormulaParser` only ever had the one default instance. Furthermore, there are no real use cases to change its behaviour without changing the only user of it, `PreprocessorAnnotationParser`, because the string syntax couples them very extremely. Note that the `WontfixTestCases` are actually parsed correctly by now.
Replacing special characters in variable names is mostly necessary to adhere to the grammar supported by the FeatureIDE parser for `Node`s. However, we already do our own parsing using Antlr4 so we can skip this step by building the `Node` tree our selves. This also removes one more source of bugs because overall there is less code involved. Beware that this introduces a semantic change: The new JPP parser abstracts `!defined(property)` to a negative literal `defined(property)` instead of a positive literal with a prefix denoting the negation.
This class is weirdly coupled with its subclasses. It uses a regex and assumes that some arbitrary groups exist and have the correct meaning. Furthermore, the exception handling handles UncheckedUnParseableFormulaException specially which should also be an implementation detail of the code that actually throws it. Last but not least, it only saves redundant exception handling code (which is dependent on the subclass) as the matching code has to be repeated by CPPDiffLineFormulaExtractor anyways (also degrading performance as the regex needs to be matched twice for no reason).
This makes it consistent with UnparseableFormulaException.
The old method also didn't reproduce the same environment if there was a cache hit (the sandbox was disabled in this case).
This should be more reliable than the old cache-install action because all the software is operated according to their intended interfaces.
After testing (everything works as excepted) and measuring the cache, we noticed that the time improvement is not worth the additional complexity. Hence, we decided to remove this cache but leave it in the commit history in case it becomes relevant in the future. This effectively reverts commit da11a13 "CI: Cache dependencies between runs".
In case the Nix sandbox is enabled, the github-metadata plugin automatically disables network accesses. However, if the sandbox is disabled (e.g., on MacOS or by explicitly disabling it), the github-metadata plugin tries to access the GitHub API and (fortunately) fails which aborts the build. As the required patch is currently unreleased, the patch is directly fetched from GitHub. This overlay can be removed as soon as this patch lands in nixpkgs and we update to such a nixpkgs version.
Fix CI cache restore
Improve boolean abstraction
The difference between the two created Jars should be inferable without looking at the documentation.
This is a remnant of building the docker container using Nix and was used to reduce the size of the docker container. For our current usage, this only increases complexity, brittleness and reduces maintainability.
Due to readability/understandability issues and the hardness for static analyzers, the usage of the `with` syntax is generally discouraged nowadays.
This shouldn't make a real difference but makes this a little bit more explicit and maybe even more robust.
This makes overrides easier in my experience.
Credits for this approach go to Peter Kolloch. See his blog entry about this idea: https://blog.eigenvalue.net/nix-rerunning-fixed-output-derivations/
During the update of dependencies, Maven advised to do this. Version 3.6.3 of Maven is technically already "end of life" but, "currently, plugins provide Maven API compatibility down to 3.6.3" (https://maven.apache.org/docs/history.html 205-06-04) so we do this too. Note that we do not currently have any requirement to use any later version so this is totally fine.
Merged
Update dependencies
Lines which aren't comments are identified beforehand and thus never passed to this JPP parser. Fortunately, the comments suggest the intend which the new test cases should now implement.
Until 5056de8, the Marlin macros `ENABLED` and `DISABLED` where handled specially to extract more useful feature mappings.
Fix marlin macro interpretation
The `GitDiffer` class doesn't own (in the sense that the actual truth of these members lies in the `Repository` class) any of its members. Furthermore, it just bundles the same members as `Repository`, just leaving those out, that are unneeded. The constructor of `GitDiffer` even extracts these members from `Repository` and doesn't even allow them to be past any other way. Furthermore, most of the interesting functions are already static anyways. A different approach would be to move all of methods into `Repository`. However, there seems to be no reason to override any of the actual diffing methods and the drawback would be a huge `Repository` class which would serve a second purpose.
Abstract `git log` calls
Since 36dc1bb, commits filtered by `DiffFilter` were not counted towards the total commit count. Furthermore, `forSingleCommit` reported two total commits.
Fix the total commit count
The dependency update of the maven-javadoc-plugin in afd65e1 removed the possibility to define an alternative output directory for the generated javadoc which resulted in a warning that I missed until now. This regression was intentional and is declared as won't fix by upstream. See apache/maven-javadoc-plugin#1194
Merged
Fix javadoc deployment
I checked all hand written text file extensions in use and most of them already use 4 space indent majorly. The only exceptions are *.sh, which is a wild chaos were any arbitrary decision will do, and *.nix where two space indenting is prevalent in the Nix community (i.e., Nixpkgs). See https://editorconfig.org/ for the specification and the goals of `.editorconfig` files.
If you try to blame, log or similar and hit this commit, use `-w` to skip commits like this. For example `git blame -w`.
Create a `.editorconfig` and apply it to all source files
Member
Author
|
@ibbem Could you increase the version number and accept this PR for merge? (I do not think it is actually necessary to re-review everything again, since we already did that as part of all the individual PRs.) |
ibbem
approved these changes
Jul 2, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
FormulaUtilswhere appropriate #159DiffLineFormulaExtractor#160