Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-41526

"Filter for Poll SCM" detects changes on similarly named folders

    • Icon: Bug Bug
    • Resolution: Fixed
    • Icon: Major Major
    • accurev-plugin
    • Jenkins Version 2.32.1
      AccuRev Plugin Version 0.7.9

      "Filter for Poll SCM" detects changes on similarly named folders.

      We have three folders in the root of the stream named "hi-lib", "hi-lib-dal", and "hi-lib-dal-mongo". The "Filter for Poll SCM" for each folder job is set as "hi-lib/", "hi-lib-dal/", and "hi-lib-dal-mongo/". This is because without the trailing forward-slash AccuRev Plugin Version 0.7.6 would detect changes in each job even if changes were only promoted to one folder - adding the forward-slash resolved that issue.

      With AccuRev Plugin Version 0.7.9, after promoting a change to "hi-lib-dal-mongo", all three builds started for detected changes in all three folders even though only "hi-lib-dal-mongo" contained a promoted change.

      It appears something changed in AccuRev Plugin Version 0.7.9 that broke the polling and filtering by folders by ignoring the forward-slash that denoted the end of the folder name. Is there another way I should qualify the "Filter for Poll SCM" value? Thanks.

          [JENKINS-41526] "Filter for Poll SCM" detects changes on similarly named folders

          Joseph Petersen (old) added a comment - - edited

          will simple wildcard work?

          so in your case it would be for each job. They can still be comma seperated if you need more filters

          *hi-lib/*
          *hi-lib-dal/*
          *hi-lib-dal-mongo/*
          

          ?
          or perhaps targeting both main and test scope

          *src/*/java/* 
          

          Your welcome to take a look at the current implementation and come with a PR
          https://github.com/jenkinsci/accurev-plugin/blob/master/src/main/java/hudson/plugins/accurev/CheckForChanges.java#L101

          This is my current implementation of simple wildcard
          https://github.com/casz/accurev-plugin/blob/master/src/main/java/hudson/plugins/accurev/CheckForChanges.java#L103

          Joseph Petersen (old) added a comment - - edited will simple wildcard work? so in your case it would be for each job. They can still be comma seperated if you need more filters *hi-lib/* *hi-lib-dal/* *hi-lib-dal-mongo/* ? or perhaps targeting both main and test scope *src/*/java/* Your welcome to take a look at the current implementation and come with a PR https://github.com/jenkinsci/accurev-plugin/blob/master/src/main/java/hudson/plugins/accurev/CheckForChanges.java#L101 This is my current implementation of simple wildcard https://github.com/casz/accurev-plugin/blob/master/src/main/java/hudson/plugins/accurev/CheckForChanges.java#L103

          arnom Since you refactored it last, perhaps you have a better suggestion on the issue?

          Joseph Petersen (old) added a comment - arnom Since you refactored it last, perhaps you have a better suggestion on the issue?

          Arno Moonen added a comment -

          The filter is currently pretty.. well, ugly. It simply checks if the path contains the strings from the filter list.
          I did not think of this particular use case when refactoring the code, otherwise I would not have included the / in the STRIP_CHARS (see line 133 of CheckForChanges.java).
          I'm in favor of removing the STRIP_CHARS variable and calling StringUtils.stripAll with only a single argument.
          This will only strip all the whitespace from the strings.

          I would also propose to change line 110 in CheckForChanges.java, replacing the call to StringUtils.indexOfAny with StringUtils.startsWithAny.
          However, we should make sure that the "root" of the paths is the same in all conditions.

          It would be even better to allow for the "Ant-style" include patterns (which would allow for wildcards and such), but I would have to look into that.

          Arno Moonen added a comment - The filter is currently pretty.. well, ugly. It simply checks if the path contains the strings from the filter list. I did not think of this particular use case when refactoring the code, otherwise I would not have included the / in the STRIP_CHARS (see line 133 of CheckForChanges.java ). I'm in favor of removing the STRIP_CHARS variable and calling StringUtils.stripAll with only a single argument. This will only strip all the whitespace from the strings. I would also propose to change line 110 in CheckForChanges.java , replacing the call to StringUtils.indexOfAny with StringUtils.startsWithAny . However, we should make sure that the "root" of the paths is the same in all conditions. It would be even better to allow for the "Ant-style" include patterns (which would allow for wildcards and such), but I would have to look into that.

          I looked into ant style most of the implementation requires that the files exist. Which in this case they do not.
          So all I could do was simple wildcard without implementing too much.

          I checked and server paths given are based on folders and files structure of the current stream

          Joseph Petersen (old) added a comment - I looked into ant style most of the implementation requires that the files exist. Which in this case they do not. So all I could do was simple wildcard without implementing too much. I checked and server paths given are based on folders and files structure of the current stream

          Verified appending * to polling folder path resolved this issue. Verified with version 0.7.11.

          Timothy Williams added a comment - Verified appending * to polling folder path resolved this issue. Verified with version 0.7.11.

            jetersen Joseph Petersen
            mitorez Timothy Williams
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

              Created:
              Updated:
              Resolved: