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

Persist Binding across DynamicParser script execs #46

Merged
merged 1 commit into from Jan 17, 2015

Conversation

@Vynce
Copy link
Contributor

Vynce commented Oct 27, 2014

Don't create a new Binding for every match/execution of the
DynamicParser script. This allows the script to add new variables to the
Binding and have them persist across all matches. Variables can be added
to the Binding within a script by omitting a type [1]. The Binding can
be checked for the presence of global variables with something like [2]:
binding.variables.containsKey("bindingVar"). Also add a reference to the
falsePositive Warning object. The motivation for these changes is to
enable scripts to keep a reference to the current working directory that
can be used to resolve relative paths in warning messages.

An example script that makes use of this feature to keep track of the
current working directory follows:

// This script is designed to operate on Windows paths in a Unix
// environment. Since java.nio.file only appears to support local
// filesystems supported by the JVM, the Windows path separators need
// to be changed manually and some of the Path methods don't function
// correctly (like isAbsolute()).

// Associated regular expression:
// ^((.+?):(\d+):(?:(\d+):)? (warning|.*error): (.*))|
//  (Waf: Entering directory\s*\`(.*)\')$

import hudson.plugins.warnings.parser.Warning
import hudson.plugins.analysis.util.model.Priority
import java.nio.file.Path
import java.nio.file.Paths

// If it matched a directory
if(matcher.group(7) != null) {
    // Save the directory in the Binding as a Path object
    directory = Paths.get(matcher.group(8).replace("\\", "/"))
    return falsePositive
}

String fileName = matcher.group(2).replace("\\", "/")
int lineNumber = Integer.parseInt(matcher.group(3))
int column = Integer.parseInt(matcher.group(4))
String message = matcher.group(6)

Priority priority
String category

if (matcher.group(5).contains("error")) {
    priority = Priority.HIGH
    category = "Error"
}
else {
    priority = Priority.NORMAL
    category = "Warning"
}

// If the fileName is not absolute and we
// have a directory variable in the Binding,
if(!fileName.contains(":") &&
   binding.variables.containsKey("directory")) {
    // then append the filename to the directory and normalize
    // the path to remove redundant components
    fileName = directory.resolve(fileName).normalize().toString()
}

Warning warning = new Warning(fileName, lineNumber,
                              "Relative GCC Parser", category,
                              message, priority)
warning.setColumnPosition(column)
return warning
Don't create a new Binding for every match/execution of the
DynamicParser script. This allows the script to add new variables to the
Binding and have them persist across all matches. Variables can be added
to the Binding within a script by omitting a type [1]. The Binding can
be checked for the presence of global variables with something like [2]:
binding.variables.containsKey("bindingVar"). Also add a reference to the
falsePositive Warning object. The motivation for these changes is to
enable scripts to keep a reference to the current working directory that
can be used to resolve relative paths in warning messages.

1. http://groovy.codehaus.org/Scoping+and+the+Semantics+of+%22def%22
2. http://www.justinleegrant.com/?p=129

An example script that makes use of this feature to keep track of the
current working directory follows:

  // This script is designed to operate on Windows paths in a Unix
  // environment. Since java.nio.file only appears to support local
  // filesystems supported by the JVM, the Windows path separators need
  // to be changed manually and some of the Path methods don't function
  // correctly (like isAbsolute()).

  // Associated regular expression:
  // ^((.+?):(\d+):(?:(\d+):)? (warning|.*error): (.*))|
  //  (Waf: Entering directory\s*\`(.*)\')$

  import hudson.plugins.warnings.parser.Warning
  import hudson.plugins.analysis.util.model.Priority
  import java.nio.file.Path
  import java.nio.file.Paths

  // If it matched a directory
  if(matcher.group(7) != null) {
      // Save the directory in the Binding as a Path object
      directory = Paths.get(matcher.group(8).replace("\\", "/"))
      return falsePositive
  }

  String fileName = matcher.group(2).replace("\\", "/")
  int lineNumber = Integer.parseInt(matcher.group(3))
  int column = Integer.parseInt(matcher.group(4))
  String message = matcher.group(6)

  Priority priority
  String category

  if (matcher.group(5).contains("error")) {
      priority = Priority.HIGH
      category = "Error"
  }
  else {
      priority = Priority.NORMAL
      category = "Warning"
  }

  // If the fileName is not absolute and we
  // have a directory variable in the Binding,
  if(!fileName.contains(":") &&
     binding.variables.containsKey("directory")) {
      // then append the filename to the directory and normalize
      // the path to remove redundant components
      fileName = directory.resolve(fileName).normalize().toString()
  }

  Warning warning = new Warning(fileName, lineNumber,
                                "Relative GCC Parser", category,
                                message, priority)
  warning.setColumnPosition(column)
  return warning
@Vynce

This comment has been minimized.

Copy link
Contributor Author

Vynce commented Oct 27, 2014

Let me know if you want a unit test for this. I haven't got as far as figuring out how all that works yet.

@uhafner

This comment has been minimized.

Copy link
Member

uhafner commented Oct 31, 2014

I'm just wondering what happens if the same parser is shared between different jobs. If the same binding is used across different projects this can lead to wrong results.

Also, the compiled script might get garbage collected in the meantime so the stored binding may not always be the same. (But anyway in that case we have the same situation as we currently have without your change).

uhafner added a commit that referenced this pull request Jan 17, 2015
Persist Binding across DynamicParser script execs
@uhafner uhafner merged commit 39158da into jenkinsci:master Jan 17, 2015
1 check passed
1 check passed
default This pull request looks good
Details
@Vynce

This comment has been minimized.

Copy link
Contributor Author

Vynce commented Jan 23, 2015

Thanks for merging this. If it makes you feel more comfortable, we've been running with this change for a couple of months with several concurrent jobs on several slaves without any problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.