Plugin does not build against the current gerrit HEAD about gerrit-owners HOT 13 CLOSED

This was broken at least starting with gerrit 2.9

This plugin still builds for me with gerrit 2.8.x AFAICT.

from gerrit-owners.

Hey Niko,

you should try to see if the patch works for you.
I expect that it will cause a bunch of warnings, but the resulting jars may very well just work.

Can you test it and report back?

From 92d371be3bec5f4d6ca8525bfbc8a1f82eaf9881 Mon Sep 17 00:00:00 2001
From: Ugo Di Girolamo <[email protected]>
Date: Mon, 3 Nov 2014 23:39:55 -0500
Subject: [PATCH] Experimental fix for gerrit-owners to build vs gerrit's HEAD

current gerrit's head is 7a05f9f
---
 common.defs                   | 55 +++++++++++++------------------------------
 gerrit-owners-autoassign/BUCK |  2 ++
 gerrit-owners-common/BUCK     | 40 +++++++++++++++++++++++++++----
 gerrit-owners/BUCK            |  9 ++++---
 4 files changed, 58 insertions(+), 48 deletions(-)

diff --git a/common.defs b/common.defs
index 17f4c86..a717515 100644
--- a/common.defs
+++ b/common.defs
@@ -1,44 +1,14 @@
 include_defs('//lib/maven.defs')
 include_defs('//lib/prolog/prolog.defs')

-JACKSON_REV = '2.1.1'
-maven_jar(
-  name = 'jackson-core',
-  id = 'com.fasterxml.jackson.core:jackson-core:%s' % JACKSON_REV,
-  license = 'Apache2.0',
-)
-
-maven_jar(
-  name = 'jackson-databind',
-  id = 'com.fasterxml.jackson.core:jackson-databind:%s' % JACKSON_REV,
-  license = 'Apache2.0',
-)
-
-maven_jar(
-  name = 'jackson-annotations',
-  id = 'com.fasterxml.jackson.core:jackson-annotations:%s' % JACKSON_REV,
-  license = 'Apache2.0',
-)
-
-maven_jar(
-  name = 'jackson-dataformat-yaml',
-  id = 'com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:%s' % JACKSON_REV,
-  license = 'Apache2.0',
-)
-
-maven_jar(
-  name = 'gitective-core',
-  id = 'org.gitective:gitective-core:0.9.9',
-  license = 'Apache2.0',
-)
-
 EXTERNAL_DEPS = [
-  ':gitective-core',
-  ':jackson-core',
-  ':jackson-databind',
-  ':jackson-annotations',
-  ':jackson-dataformat-yaml',
-]
+  '//plugins/gerrit-owners/gerrit-owners-common' + i for i in [
+    ':gitective-core',
+    ':jackson-core',
+    ':jackson-databind',
+    ':jackson-annotations',
+    ':jackson-dataformat-yaml',
+  ]]

 # These are dependencies that must be made available to the plugins' libraries at compilation
 # time, but should not be included in the plugins' jar files since they will be provided
@@ -46,6 +16,15 @@ EXTERNAL_DEPS = [
 # For this reason all the intermediate java libraries that we build are java_library2 targets
 # rather than java_library.
 COMPILE_DEPS = [
-  '//:plugin-lib',
+  '//lib:guava',
+  '//lib/guice:guice',
+  '//lib:gwtorm',
+  '//lib/jgit:jgit',
+  '//lib/log:api',
+  '//gerrit-plugin-api:plugin-api',
+  '//gerrit-reviewdb:client',
+  '//gerrit-reviewdb:server',
+  '//gerrit-server:server',
+  '//gerrit-extension-api:lib',
   '//lib/prolog:prolog-cafe',
 ]
diff --git a/gerrit-owners-autoassign/BUCK b/gerrit-owners-autoassign/BUCK
index 18d0393..2678ed9 100644
--- a/gerrit-owners-autoassign/BUCK
+++ b/gerrit-owners-autoassign/BUCK
@@ -1,4 +1,6 @@
 include_defs('//plugins/gerrit-owners/common.defs')
+include_defs('//lib/maven.defs')
+include_defs('//lib/prolog/prolog.defs')

 gerrit_plugin(
   name = 'owners-autoassign',
diff --git a/gerrit-owners-common/BUCK b/gerrit-owners-common/BUCK
index ea5fb6a..61f5e60 100644
--- a/gerrit-owners-common/BUCK
+++ b/gerrit-owners-common/BUCK
@@ -1,14 +1,44 @@
 include_defs('//plugins/gerrit-owners/common.defs')
+include_defs('//lib/maven.defs')
+include_defs('//lib/prolog/prolog.defs')
+
+JACKSON_REV = '2.1.1'
+maven_jar(
+  name = 'jackson-core',
+  id = 'com.fasterxml.jackson.core:jackson-core:%s' % JACKSON_REV,
+  license = 'Apache2.0',
+)
+
+maven_jar(
+  name = 'jackson-databind',
+  id = 'com.fasterxml.jackson.core:jackson-databind:%s' % JACKSON_REV,
+  license = 'Apache2.0',
+)
+
+maven_jar(
+  name = 'jackson-annotations',
+  id = 'com.fasterxml.jackson.core:jackson-annotations:%s' % JACKSON_REV,
+  license = 'Apache2.0',
+)
+
+maven_jar(
+  name = 'jackson-dataformat-yaml',
+  id = 'com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:%s' % JACKSON_REV,
+  license = 'Apache2.0',
+)
+
+maven_jar(
+  name = 'gitective-core',
+  id = 'org.gitective:gitective-core:0.9.9',
+  license = 'Apache2.0',
+)

 # see common.defs on why this is a java_library2 rather than java_library
-java_library2(
+java_library(
   name = 'common',
   srcs = glob([
     'src/main/java/**/*.java',
   ]),
-  deps = [],
-  # Notice that we put EXTERNAL_DEPS as compile_deps to prevent double inclusion in the owners
-  # plugin.
-  compile_deps = COMPILE_DEPS + EXTERNAL_DEPS,
+  deps = COMPILE_DEPS + EXTERNAL_DEPS,
   visibility = ['PUBLIC'],
 )
diff --git a/gerrit-owners/BUCK b/gerrit-owners/BUCK
index 0ef0040..c425c65 100644
--- a/gerrit-owners/BUCK
+++ b/gerrit-owners/BUCK
@@ -1,21 +1,20 @@
 include_defs('//plugins/gerrit-owners/common.defs')
+include_defs('//lib/maven.defs')
+include_defs('//lib/prolog/prolog.defs')

 # buck is unhappy to build a gerrit_plugin with no source files.
 # On the other side, gerrit-owners-prolog-rules needs to be built after all the java files have been
 # compiled. For this reason we have a stub class that has no content and just makes buck happy.
 COMPILE_STUB = ['src/main/java/com/vmware/gerrit/owners/CompileStub.java']

-java_library2(
+java_library(
   name = 'gerrit-owners-lib',
   srcs = glob([
     'src/main/java/**/*.java',
   ], excludes=COMPILE_STUB),
   deps = [
     '//plugins/gerrit-owners/gerrit-owners-common:common',
-  ],
-  # Notice that we put EXTERNAL_DEPS as compile_deps to prevent double inclusion in the owners
-  # plugin.
-  compile_deps = COMPILE_DEPS + EXTERNAL_DEPS,
+  ] + COMPILE_DEPS + EXTERNAL_DEPS,
 )

 prolog_cafe_library(
--
2.1.1

from gerrit-owners.

Hi Ugo,

I applied the patch and the jar built fine (the warnings you mentioned were for duplicate java classes). Thank you! Feel free to commit your change and I can do a fresh clean pull.

from gerrit-owners.

Hi,

I just tested with this patch in gerrit 2.10. It can successfully generate the plugin jar file owners.jar.
However, when trying to put the jar into gerrit/plugins/ folder, it shows error:

[2015-03-04 16:48:34,900] WARN com.google.gerrit.server.plugins.PluginLoader : Plugin owners tries to @null("null") abstract class com.google.gerrit.rules.PrologEnvironment$Factory
[2015-03-04 16:48:34,900] WARN com.google.gerrit.server.plugins.PluginLoader : Plugin owners tries to @null("null") abstract class com.google.gerrit.rules.PredicateProvider
[2015-03-04 16:48:34,900] WARN com.google.gerrit.server.plugins.PluginLoader : Plugin owners tries to @null("null") abstract class com.google.gerrit.rules.PrologCompiler$Factory
[2015-03-04 16:48:34,901] WARN com.google.gerrit.server.plugins.PluginLoader : Plugin owners tries to @null("null") abstract class com.google.gerrit.audit.AuditListener
[2015-03-04 16:48:35,029] WARN com.google.gerrit.server.plugins.PluginLoader : Cannot load plugin owners
com.google.gerrit.server.plugins.InvalidPluginException: Cannot register com.google.gerrit.server.securestore.DefaultSecureStore, server does not accept com.google.gerrit.server.securestore.SecureStore
at com.google.gerrit.server.plugins.AutoRegisterModules.listen(AutoRegisterModules.java:216)
at com.google.gerrit.server.plugins.AutoRegisterModules.listen(AutoRegisterModules.java:226)
at com.google.gerrit.server.plugins.AutoRegisterModules.export(AutoRegisterModules.java:151)
at com.google.gerrit.server.plugins.AutoRegisterModules.scan(AutoRegisterModules.java:115)
at com.google.gerrit.server.plugins.AutoRegisterModules.discover(AutoRegisterModules.java:76)
at com.google.gerrit.server.plugins.ServerPlugin.startPlugin(ServerPlugin.java:180)
at com.google.gerrit.server.plugins.ServerPlugin.start(ServerPlugin.java:167)
at com.google.gerrit.server.plugins.PluginLoader.runPlugin(PluginLoader.java:460)
at com.google.gerrit.server.plugins.PluginLoader.rescan(PluginLoader.java:389)
at com.google.gerrit.server.plugins.PluginLoader.start(PluginLoader.java:294)
at com.google.gerrit.lifecycle.LifecycleManager.start(LifecycleManager.java:74)
at com.google.gerrit.pgm.Daemon.start(Daemon.java:291)
at com.google.gerrit.pgm.Daemon.run(Daemon.java:203)
at com.google.gerrit.pgm.util.AbstractProgram.main(AbstractProgram.java:64)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:606)
at com.google.gerrit.launcher.GerritLauncher.invokeProgram(GerritLauncher.java:166)
at com.google.gerrit.launcher.GerritLauncher.mainImpl(GerritLauncher.java:93)
at com.google.gerrit.launcher.GerritLauncher.main(GerritLauncher.java:50)
at Main.main(Main.java:25)

from gerrit-owners.

Hey Bevin, haven't touched this in a little bit, but I will have to upgrade my own gerrit sometime in the near future. I hope to test this over the weekend and make it functional with gerrit's latest release.

from gerrit-owners.

So I'm currently testing a fix for this.
I have a version that build out on my fork, will test and will create a pull-request once it is working.
https://github.com/ugodiggi/gerrit-owners

from gerrit-owners.

My pull request #5 should fix this issue.

from gerrit-owners.

Hi Ugo,
I was able to install the plug-in fine this time. Thanks!

I am however using the jenkins plug-in to add a "Verified" label. What I
notice now is that instead of seeing the Verified label and the
Owner-Approval label from the owners plug-in I see instead the jenkins user
added to my reviewers automatically.

Is there some config I need to modify for the owners plug-in?

Also, one of our developers is wondering if we can do owners per file
instead of per directory. Is that possible?

thanks again for fixing this.

On Wed, Apr 1, 2015 at 5:21 PM Ugo Di Girolamo [email protected]
wrote:

My pull request #5 #5
should fix this issue.

—
Reply to this email directly or view it on GitHub
#4 (comment).

from gerrit-owners.

My bad. I remembed you had sent me instructions to add a rules.pl file and
troubleshoot. Let me try that before I waste your time about the label
issue.
I'd still be interested to know if there is a way to do ownership per file.

On Wed, Apr 15, 2015 at 11:15 AM Nikolaos Michalakis <
[email protected]> wrote:

Hi Ugo,
I was able to install the plug-in fine this time. Thanks!

I am however using the jenkins plug-in to add a "Verified" label. What I
notice now is that instead of seeing the Verified label and the
Owner-Approval label from the owners plug-in I see instead the jenkins user
added to my reviewers automatically.

Is there some config I need to modify for the owners plug-in?

Also, one of our developers is wondering if we can do owners per file
instead of per directory. Is that possible?

thanks again for fixing this.

On Wed, Apr 1, 2015 at 5:21 PM Ugo Di Girolamo [email protected]
wrote:

My pull request #5 #5
should fix this issue.

—
Reply to this email directly or view it on GitHub
#4 (comment).

from gerrit-owners.

Update: after I used the rules.pl from the README I got this to work as
advertised (i.e., ownership per directory).

On Wed, Apr 15, 2015 at 11:32 AM Nikolaos Michalakis <
[email protected]> wrote:

My bad. I remembed you had sent me instructions to add a rules.pl file
and troubleshoot. Let me try that before I waste your time about the label
issue.
I'd still be interested to know if there is a way to do ownership per file.

On Wed, Apr 15, 2015 at 11:15 AM Nikolaos Michalakis <
[email protected]> wrote:

Hi Ugo,
I was able to install the plug-in fine this time. Thanks!

I am however using the jenkins plug-in to add a "Verified" label. What I
notice now is that instead of seeing the Verified label and the
Owner-Approval label from the owners plug-in I see instead the jenkins user
added to my reviewers automatically.

Is there some config I need to modify for the owners plug-in?

Also, one of our developers is wondering if we can do owners per file
instead of per directory. Is that possible?

thanks again for fixing this.

On Wed, Apr 1, 2015 at 5:21 PM Ugo Di Girolamo [email protected]
wrote:

My pull request #5 #5
should fix this issue.

—
Reply to this email directly or view it on GitHub
#4 (comment)
.

from gerrit-owners.

I'm not super fresh with the code, so everything is iirc.
The code currently is only supporting directory level ownership, the bulk of the code is in:

https://github.com/vadims/gerrit-owners/blob/master/gerrit-owners-common/src/main/java/com/vmware/gerrit/owners/common/PathOwners.java
and of course the ownership model is https://github.com/vadims/gerrit-owners/blob/master/gerrit-owners-common/src/main/java/com/vmware/gerrit/owners/common/OwnersConfig.java

I think it would be easy-ish to change the code to add ownership by file.
Personally I would not want to have such a feature in my code base, though, since I prefer separating in multiple directories.

from gerrit-owners.

I'm not sure about per file OWNERS, but at the time we considered adding support for OWNERS per language..

inherited: true
owners:
- [email protected]
- [email protected]
files:
- pattern: *.java
  owners:
    - [email protected]
- pattern: *.js
  owners:
  - [email protected]

from gerrit-owners.

Thanks Ugo.

I just wanted to know if you supported it. That's all. We have a fully
working gerrit with owners now so I'm good :)

Thanks for your help guys.

nikos

On Wed, Apr 15, 2015 at 12:13 PM Ugo Di Girolamo [email protected]
wrote:

I'm not super fresh with the code, so everything is iirc.
The code currently is only supporting directory level ownership, the bulk
of the code is in:

https://github.com/vadims/gerrit-owners/blob/master/gerrit-owners-common/src/main/java/com/vmware/gerrit/owners/common/PathOwners.java
and of course the ownership model is
https://github.com/vadims/gerrit-owners/blob/master/gerrit-owners-common/src/main/java/com/vmware/gerrit/owners/common/OwnersConfig.java

I think it would be easy-ish to change the code to add ownership by file.
Personally I would not want to have such a feature in my code base,
though, since I prefer separating in multiple directories.

—
Reply to this email directly or view it on GitHub
#4 (comment).

from gerrit-owners.