New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 832920 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: ----



Sign in to add a comment

Copyright analyzer not running even though config has been set up

Project Member Reported by qyears...@chromium.org, Apr 13 2018

Issue description

The "Copyright" analyzer has just been added to the list of selections in fuchsia project configs in https://fuchsia-review.googlesource.com/c/infra/config/+/140022.

It now appears that it should be enabled, as the configs have all been updated:

https://luci-config.appspot.com/#/projects/fuchsia-garnet
https://luci-config.appspot.com/#/projects/fuchsia-peridot
https://luci-config.appspot.com/#/projects/fuchsia-topaz
https://luci-config.appspot.com/#/projects/fuchsia-zircon

https://luci-config.appspot.com/#/services/tricium-prod

But, in actual recent CLs, there don't appear to be any Copyright analyzer runs yet.

Possible hypothesis #1: Maybe our PathFilters logic doesn't work quite right.
 
Owner: qyears...@chromium.org
Status: Assigned (was: Available)
I didn't get time yesterday or today to look into this, and I'll be busy for the next couple days, but I think this is important and will try to investigate and update if possible.
Status: Started (was: Assigned)
Just took a look and confirmed that path filters don't work quite as we want them to; uploaded https://crrev.com/c/1023595.
Project Member

Comment 3 by bugdroid1@chromium.org, Apr 23 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra/+/d623320fabd9be1a62e3cb1414e67e6bb71a0c10

commit d623320fabd9be1a62e3cb1414e67e6bb71a0c10
Author: Quinten Yearsley <qyearsley@chromium.org>
Date: Mon Apr 23 17:21:55 2018

Change PathFilter matching to apply only to file basenames

Path filters are matched against paths with filepath.Match;
this matching works like Bash glob matching, so
foo/*.md matches "foo/bar.md", but "*.md" does not.

But, for analyzer path filters, we mainly want to use path
filters to be able to express "this analyzer applies to a
certain type of file.".

This CL changes the behavior so that "*.ext" matches files
with that extension regardless of directory nesting.

Bug:  832920 
Change-Id: I3c7456303fcd5ff4bff2bd6e1ba949bd9aa1f53a
Reviewed-on: https://chromium-review.googlesource.com/1023595
Commit-Queue: Quinten Yearsley <qyearsley@chromium.org>
Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org>

[modify] https://crrev.com/d623320fabd9be1a62e3cb1414e67e6bb71a0c10/go/src/infra/tricium/api/v1/pb.discovery.go
[modify] https://crrev.com/d623320fabd9be1a62e3cb1414e67e6bb71a0c10/go/src/infra/tricium/api/v1/function.pb.go
[modify] https://crrev.com/d623320fabd9be1a62e3cb1414e67e6bb71a0c10/go/src/infra/tricium/appengine/common/config/generate_test.go
[modify] https://crrev.com/d623320fabd9be1a62e3cb1414e67e6bb71a0c10/go/src/infra/tricium/api/v1/function.proto
[modify] https://crrev.com/d623320fabd9be1a62e3cb1414e67e6bb71a0c10/go/src/infra/tricium/appengine/common/config/generate.go

After that change, it looks like the copyright analyzer is running (at least in the "playground" repo: https://chromium-review.googlesource.com/c/playground/gerrit-tricium/+/1025141
It's still not running in the Fuchsia repos: https://fuchsia-review.googlesource.com/c/garnet/+/146924
Status: Fixed (was: Started)
Alright, thanks for confirming :-)

Note, it wasn't run on https://fuchsia-review.googlesource.com/c/garnet/+/146924 because that change doesn't contain any paths that match the path filters. (Maybe a few more file extensions could potentially be added to the path filters, e.g. "*.go".)

Sign in to add a comment