Support of config bundles with extra configuration files #219

relentless-pursuit · 2024-11-12T17:54:21Z

Part of: #214

romani

Items

romani · 2024-11-13T16:39:43Z

extractor/src/test/java/com/example/extractor/MainsLauncherTest.java

+
+        // Verify that the generated config includes the external file reference
+        assertThat(generatedConfigContent)
+                .contains("<property name=\"headerFile\" value=\"config/java.header\"/>");


We need path that will work for diff tool.
I think we can value=\"${execution.path}/config/java.header\"/>"
And we will set this variable when we start diff tool, so internal checkstyle execution will not have problems to find this header file.

extractor/src/test/java/com/example/extractor/MainsLauncherTest.java

relentless-pursuit · 2024-11-27T04:49:14Z

@romani, please review.

romani · 2024-11-29T16:16:53Z

@relentless-pursuit , please disable execution of cron action in your fork, to avoid extra commits.

romani

Items

romani · 2024-11-29T16:40:54Z

extractor/src/main/java/com/example/extractor/CheckstyleExampleExtractor.java

@@ -147,6 +172,19 @@ public static void main(final String[] args) throws Exception {
            // Output default projects list
            outputDefaultProjectsList(projectsOutputPath);
        }
+        else if (args.length == CONFIG_FILE_ARG_COUNT
+                && "--config-file".equals(args[CONFIG_FILE_FLAG_INDEX])) {


It should not be any different from regular execution on Input.
We should have a some structure inside that describe all Checks properties that having external property files. We do not have much of them, better to hard code them for now.

We might need to simply update examples in main repository to reference same config name and use the same system variable and path in config.
So we can simply put a predefined file to generation target folder.

If some module define content of config and we need to use it, we can rename them to be predictable and just use specific logic for module that is registered as external file module.

https://checkstyle.org/checks/header/regexpheader.html#Example2-config

We can make it:

<module name="Checker"> <module name="RegexpHeader"> <property name="headerFile" value="${config.path}/java.header"/> <property name="multiLines" value="10, 13"/> </module> </module>

in main repository.

And in test-configs project just parse Input as regular Input, if we detect special module like this, we copy config files that are hard coded path to them in our code structure.

During execution of test bundle we just always define this system variable, even it will not be in use by module.

@relentless-pursuit , ping. Do you need any assistance?

@relentless-pursuit , ping. Do you need any assistance?

@romani, can you re-review?

romani

items:

romani · 2024-12-05T16:47:04Z

extractor/src/main/java/com/example/extractor/CheckstyleExampleExtractor.java

+        String updatedContent = configContent;
+
+        // Regular expression to find properties with external file references
+        final Pattern pattern = Pattern.compile("<property name=\"([^\"]+)\" value=\"([^\"]+)\"/>");


lets simply hardcode in this project properties with files, there only few of them, and unlikely to grow a lot.

romani · 2024-12-05T16:48:29Z

extractor/src/main/java/com/example/extractor/CheckstyleExampleExtractor.java

+        // Replace the path with ${config.path}
+        final String newPropertyValue = propertyValue
+                .replace("${execution.path}", CONFIG_PATH_VARIABLE)
+                .replace("config/java.header", CONFIG_PATH_VARIABLE + "/java.header");


we do not need to hack path in extractor.

We can put this variable to ExampleX in checkstyle repo.

romani · 2024-12-05T16:50:03Z

extractor/src/test/java/com/example/extractor/MainsLauncherTest.java

+        // Write the java.header file inside the 'config' directory
+        final String headerFileContent = """
+        ^// Copyright \\(C\\) (\\d\\d\\d\\d -)? 2004 MyCompany$
+        ^// All rights reserved$
+            """;


please make it as static file in repo, that you will use during test execution.
same for config file.

romani · 2024-12-05T16:55:05Z

extractor/src/main/java/com/example/extractor/CheckstyleExampleExtractor.java

+                processInputFile(inputFile, configOutputFile);
+                outputDefaultProjectsList(projectsOutputFile.toString());
+            }
+            else if ("--config-file".equals(option)) {


we do not need any few argument in CLI.
as we come to extraction of Check that is in our hardcoded map/collection of modules that have external files, we just put specific file to result folder.
path to it defined by variable, variable already in config for ExampleX.java.

During execution of regression testing we just need to set varable to required value, and download such extra file.

romani · 2024-12-09T16:30:32Z

@relentless-pursuit , CI is red.

romani

Items

romani · 2024-12-09T16:31:24Z

RegexpHeader/Example2/config.xml

@@ -17,7 +17,7 @@
    </module>

    <module name="RegexpHeader">
-        <property name="headerFile" value="config/java.header"/>
+        <property name="headerFile" value="java.header"/>


Please send PR to checkstyle project

@romani , finally got the CI working. There is somethign wrong with local code base, not being able to see failures, need to delete and clone fresh, please re-review.

Ok, but still this change is not expected.

Please send update to main repository
<property name="headerFile" value="${config.folder}/java.header"/>

Extractor should not do any changes.
Config should stay unchanged.

GitHub workflow should define system variable config.folder and propagate it to checkstyle execution.

relentless-pursuit · 2024-12-29T14:40:19Z

updating this PR as we have decided to change the approach. made changes to the pom's template file.
https://github.com/checkstyle-gsoc/checkstyle/actions/runs/12536103229/job/34958763500

generate report phase was working.
@romani, can you review?

…ion files

romani

good.

item:

romani · 2024-12-29T15:08:38Z

diff-java-tool/src/main/resources/pom_template.xml

@@ -16,6 +16,7 @@
        <sevntu-checkstyle.version>1.44.1</sevntu-checkstyle.version>
        <checkstyle.config.location>https://raw.githubusercontent.com/checkstyle/checkstyle/master/src/main/resources/google_checks.xml</checkstyle.config.location>
        <checkstyle.failsOnError>true</checkstyle.failsOnError>
+        <config.folder>${project.basedir}/config</config.folder>


an we avoid this default value ?
or please explain why have it.

@romani
We store a java.header file under our config folder. Without the config.folder property and its reference, Checkstyle might not know where to locate java.header—it would either default to a different configuration or fail if it cannot find a valid header. By explicitly setting config.folder=${project.basedir}/config and using in the Checkstyle plugin, we ensure that our local java.header is always picked up during the build.

Please correct me if i am wrong or missing anything.

Does it make more sense to rely on the project for the defaults?

For 3rd party projects, we define a properties file with basically these defaults.
https://github.com/sevntu-checkstyle/sevntu.checkstyle/blob/f01cb81dd1bb771f8471196a7787630f2a84861e/sevntu-checkstyle-sonar-plugin/checkstyle.properties

Main repo doesn't have this file and it is embedded into the ant process.
https://github.com/checkstyle/checkstyle/blob/master/config/ant-phase-verify.xml#L48

So it would require a change to main repo.

we need to download such file like we do with configs NEW_MODULE_CONFIG="${{ github.workspace }}/.ci-temp/new_module_config.xml" https://github.com/checkstyle/checkstyle/blob/d1523dab4bbd520340336912ca6974905b4f41ec/.github/workflows/regression-report.yml#L555
to execute on them https://github.com/checkstyle/checkstyle/blob/d1523dab4bbd520340336912ca6974905b4f41ec/.github/workflows/regression-report.yml#L575
java.header should be file file in test-configs repository that is downloadable together with other files and java -jar "$DIFFTOOL_JAR" should execute with this variable defined to let checkstyle find java.header

I am not sure what is /config folder ?
diff-java-tool/src/main/resources/pom_template.xml is pom.xml that will be placed to filesystem during run of java -jar "$DIFFTOOL_JAR"

Check is updated to have property #224

please proceed to update workflow.

…ion files

relentless-pursuit marked this pull request as draft November 12, 2024 17:55

romani requested changes Nov 13, 2024

View reviewed changes

relentless-pursuit force-pushed the main branch 3 times, most recently from 6de8ea6 to 5cf16c2 Compare November 26, 2024 07:06

relentless-pursuit marked this pull request as ready for review November 28, 2024 05:04

romani requested changes Nov 29, 2024

View reviewed changes

relentless-pursuit force-pushed the main branch 3 times, most recently from 4351c8a to bee1587 Compare December 5, 2024 14:58

romani requested changes Dec 5, 2024

View reviewed changes

relentless-pursuit force-pushed the main branch 3 times, most recently from 18fbd83 to e1c5904 Compare December 9, 2024 05:26

romani requested changes Dec 9, 2024

View reviewed changes

relentless-pursuit force-pushed the main branch 6 times, most recently from 0440669 to 26cbd35 Compare December 10, 2024 16:33

Issue checkstyle#214: Support of config bundles with extra configurat…

b005b7e

…ion files

relentless-pursuit force-pushed the main branch from 26cbd35 to b005b7e Compare December 29, 2024 14:47

romani requested changes Dec 29, 2024

View reviewed changes

relentless-pursuit closed this Jan 15, 2025

relentless-pursuit pushed a commit to relentless-pursuit/test-configs that referenced this pull request Jan 15, 2025

Issue checkstyle#219: Support of config bundles with extra configurat…

c93226e

…ion files

relentless-pursuit mentioned this pull request Jan 15, 2025

Support of config bundles with extra configuration files #225

Merged

relentless-pursuit pushed a commit to relentless-pursuit/test-configs that referenced this pull request Jan 15, 2025

Issue checkstyle#219: Support of config bundles with extra configurat…

860978a

…ion files

relentless-pursuit pushed a commit to relentless-pursuit/test-configs that referenced this pull request Jan 15, 2025

Issue checkstyle#219: Support of config bundles with extra configurat…

02d711f

…ion files

romani pushed a commit that referenced this pull request Jan 15, 2025

Issue #219: Support of config bundles with extra configuration files

31b895f

relentless-pursuit pushed a commit to relentless-pursuit/test-configs that referenced this pull request Jan 16, 2025

Issue checkstyle#219: Support of config bundles with extra configurat…

6d22859

…ion files

relentless-pursuit mentioned this pull request Jan 16, 2025

Issue #219: Support of config bundles with extra configuration files #226

Merged

romani pushed a commit that referenced this pull request Jan 16, 2025

Issue #219: Support of config bundles with extra configuration files

32374e3

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support of config bundles with extra configuration files #219

Support of config bundles with extra configuration files #219

relentless-pursuit commented Nov 12, 2024

romani left a comment

romani Nov 13, 2024

relentless-pursuit Nov 26, 2024

relentless-pursuit commented Nov 27, 2024

romani commented Nov 29, 2024

romani left a comment

romani Nov 29, 2024

romani Nov 29, 2024

romani Dec 4, 2024

relentless-pursuit Dec 5, 2024

romani left a comment

romani Dec 5, 2024

romani Dec 5, 2024

romani Dec 5, 2024

romani Dec 5, 2024

romani commented Dec 9, 2024

romani left a comment

romani Dec 9, 2024

relentless-pursuit Dec 10, 2024

romani Dec 10, 2024

romani Dec 10, 2024

relentless-pursuit commented Dec 29, 2024 •

edited

Loading

romani left a comment

romani Dec 29, 2024

relentless-pursuit Jan 2, 2025

rnveach Jan 2, 2025 •

edited

Loading

romani Jan 2, 2025 •

edited

Loading

romani Jan 3, 2025

Support of config bundles with extra configuration files #219

Support of config bundles with extra configuration files #219

Conversation

relentless-pursuit commented Nov 12, 2024

romani left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

relentless-pursuit commented Nov 27, 2024

romani commented Nov 29, 2024

romani left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

romani left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

romani commented Dec 9, 2024

romani left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

relentless-pursuit commented Dec 29, 2024 • edited Loading

romani left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rnveach Jan 2, 2025 • edited Loading

Choose a reason for hiding this comment

romani Jan 2, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

relentless-pursuit commented Dec 29, 2024 •

edited

Loading

rnveach Jan 2, 2025 •

edited

Loading

romani Jan 2, 2025 •

edited

Loading