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

diff.groovy: allow exclude list to come from multi-line file #588

Open
rnveach opened this issue Dec 20, 2021 · 4 comments
Open

diff.groovy: allow exclude list to come from multi-line file #588

rnveach opened this issue Dec 20, 2021 · 4 comments

Comments

@rnveach
Copy link
Member

rnveach commented Dec 20, 2021

Identified at #527 (review) ,

We should not be maintaining a 13k character singleline, especially when our own repo is trying to cut down lines to 100 characters or less. This is not easily maintainable. There is also the extra maintainability that we need a separate file with a similar list in another type of file format. We clearly need a better way.

Our projects*.properties files format needs to be changed to allow this possible. The current format is:
REPO_NAME|[local|git|hg]|URL|[COMMIT_ID]|[EXCLUDE FOLDERS]

We should update it to allow an exclude file, relative to the property file's current location.
Example: REPO_NAME|[local|git|hg]|URL|[COMMIT_ID]|[EXCLUDE FOLDERS]|[EXCLUDE FILE].
Exclude file should be completely optional. It doesn't need the final | and even if it is provided, it should allowed to be nothing meaning there is no excludes in a file to use.
As long as there is no added complexity, both excludes should be additional. Meaning if I exclude folders and provide an exclude file, then they should act like they are all combined into 1 big list.

We should update it to allow an exclude file or a list of folders.
EXCLUDE FOLDERS should identify as a file if it starts with file://$PWD. It will take this file as in the current working directory. Anything that is not identified as this file should fall back on the old behavior of list of folders.

The format of exclude file should be similar to exclude folders, with the only difference being that multiple lines is encouraged. Each new line should act like an OR or similar to , in the exclude folders.
Comments (in the form of #) should be allowed. Blank lines should be allowed too.

diff.groovy needs to be updated to process the new changes in the property file.

With the new functionality, the following 2 things also need to occur:

@romani
Copy link
Member

romani commented Dec 20, 2021

what is we reuse [EXCLUDE FOLDERS]
and recheck if content of this field is file://path-to-file and this file exists, so load content from file.

file content is multiline text files where each line is path to exclude. So groovy script needs to load conten and concatenate it to single line by ,.

Example:
openjdk16|git|https://github.com/AdoptOpenJDK/openjdk-jdk16.git|master|file://openjdk16-excudes.txt

openjdk16-excudes.txt:

**/test/langtools/jdk/javadoc/doclet/testSupplementary/C.java
**/test/hotspot/jtreg/runtime/exceptionMsgs/methodPrinting/TestPrintingMethods.java
**/test/jdk/sun/tools/jhsdb/LingeredAppWithExtendedChars.java
......

@rnveach
Copy link
Member Author

rnveach commented Dec 20, 2021

recheck if content of this field is file://path-to-file and this file exists, so load content from file

  1. Small concern was possibly breaking existing files
  2. Main concern was possible false positives as exclusions are not required to always start with **.
  3. We can't rely on absolute paths like file://path-to-file (if this was intended) as everyone will be executing in their own custom paths like CI.

file content is multiline text files where each line is path to exclude. So groovy script needs to load conten and concatenate it to single line by ,.

+1 excluding comments and blank lines.

@romani
Copy link
Member

romani commented Dec 20, 2021

yes, so sad https://stackoverflow.com/a/15193744

can we agree on file://$PWD/file.txt, so file://$PWD will be our marker to apply custom logic.

@rnveach
Copy link
Member Author

rnveach commented Dec 20, 2021

I wanted to avoid custom logic so it can be used in any situation. I don't want to block this so I am ok.

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