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
Fix buildWAR target #85
Conversation
Oops. I moved the servlet while I was traveling and writing the gitdaemon support. I don't often buildWAR but eventually I would have caught it. Thanks for the patch! I did move the markdown files from being web resources to being classpath resources. I'll take a look at it tomorrow... unless you find it first. :) |
Ah, thank you for the hint. I checked the current implementation of gerrit-gitblit plugin. @singleton and have overridden the getResourceAsStream(String). @OverRide
} What i didn't understand, is what exactly Apache Wicket method doing: I will look into it further, until you solve it first ;-) Thanks |
This is now fixed. Bizarre. I remember fixing this when I moved the resources. I must have switched branches and reverted or something. As for loading reference.properties, I think we no longer need changes. The original code was a workaround for loading a "static" web resource, not a classpath resource. I assume that Gitblit-Gerrit must also subclass WicketUtils to control where context relative resources are loaded. Can you confirm? I think we can probably improve that. |
Thank you for fixing it, sorry for not seeing it and fixing it myself ;-)
Agree. i am not a friend of duplication of entire reference.properties for gerit-gitblit-plugin anyway. So currently i am trying to overwrite two properties that we actually need (and that are different from vanilla GitBlit version) and removed the whole code and reference.properties from plugin, here the updated patch: https://gerrit-review.googlesource.com/#/c/44263/ What do you think?
I guess not, because gerrit-plugin machinery does some magic here. It was working in 1.2.0 and it is still working. I've two questions (not sure if it is the best place to ask ;-)
[...] BUILD SUCCESSFUL Thanks |
reference.properties in Gitblit is not being used the same as reference.properties in Gitblit-Gerrit. Perhaps it should have been named default.properties. Gitblit-Gerrit is using it as the actual settings file, not the default/reference values. There are probably several settings that Gitblit-Gerrit will want to change from the default values. Gitblit uses defaults in code if the provided settings/properties class does not have a requested setting value.
|
Well, Gitblit-Gerrit's own com.googlesource.gerrit.plugins.gitblit.reference.properties was loaded and passed as InputStream to GitBlit.contextInitilaized() method and passed from there to loadSettingModels(), where GitBlit's own "/reference.properties" is loaded. Opps, while writing this i realized, that holds true for current gitblit.jar (based on 1.3.0-SNAPSHOT), that was built with extended moxie -DresourceFolderPrefix=static. I checked the current production gitblit.jar that was built by GerritForge: gerrit2@lvps46-163-74-91:~/gerrit_testsite/plugins$ jar -tf gitblit.jar | grep -i reference.properties So the question is still how we can keep the default.properties in GitBlit and only pass different settings to GitBlit from GitBlit-Gerrit? GitBlit war & junit: i will take a look. Thanks |
That looks like what the original Gitlbit-Gerrit code does. It loads Gerrit's gitblit settings file (reference.properties) as Gitblit's default settings. It then manually overrides a setting and passes that config to Gitblit. The last step is to pass the inputstream of Gerrit's properties file to Gitblit to load the settings models used for RPC setting management. |
This is exactly what old code GitBlit.java did that is removed now and you also wasn't amused to integrate it again with objection (that i agree with) to not pass InputStream around. So that i am asking what would be the best way to proceed here? Thanks |
Right. I was explaining what current Gitblit-Gerrit does since there is a difference between the GitBlit.settings object and the GitBlit.settingsModel object (where reference.properties is used). Read the GitBlit.loadSettingModels method to understand how reference.properties is used. I think you can restore the old code and eliminate the inputstream parameter to contextInitialized. Gitblit-Gerrit does not use Gitblit's RPC functions so it will not care about what is loaded into settingsModel. The RPC mechanism (disabled by default) is currently the only place where the settingsModel is used. Mashing Gitblit & Gerrit together is a little like putting a square peg into a round hole. With enough force you can do it - but it wasn't designed for it. |
You do want to pay attention, though, for what should be the default values for Gitblit-Gerrit. If you continue with your patch you will be using stock Gitblit GO settings, aside from the 2 you are manually setting. So either you keep a properties file for creating GitblitSettings() OR, in code, you override all relevant defaults. Some of the Gitblit GO settings may not make sense for Gerrit (RPC) or may adversely affect Gerrit. Don't ask me which ones, I don't run Gerrit. |
Thank you for these suggestions! I am going to attend Gerrit Spring Hackathon in London (7-9 May) and to meet Luca there and discuss it with him, so let us summarize the options we have:
In all these options we don't pass any InputStream(s) around ;-) What do you think? Thanks |
Looking into Tomcat gitblit.war problem: it seems that i might be missing some RTFM? Could you point me to it? tomcat log: INFO: Deploying web application archive /opt/tomcat/apache-tomcat-7.0.39/webapps/gitblit.war Thanks |
Option 4. Gitblit-Gerrit forgets "reference.properties" exists and does nothing about loading it. (My preference). I do like the idea that you should have a "gitblit-gerrit.properties" file where you configure how Gitblit-Gerrit will work, rather than just stuffing values in code. The properties file has descriptions which would be useful for other Gitblit-Gerrit contributors. It seems like the getResourcePrefix method is not needed either. WAR startup. I just built WAR and deployed to a fresh, vanilla Tomcat 7.0.39. I didn't change any settings and it worked as expected. Are you already running a GitDaemon service on this machine? |
done: https://gerrit-review.googlesource.com/#/c/44263/
Thanks |
I found the problem with WAR deployment: i started Gerrit on that machine already ... with gerrit-gitblit-plugin enabled, in which case gitblit-gerrit started already GitDaemon under the port 9418. GtBlit-War in tomcat failed to start because that port was already in use ;-) The same problem i had on another productive machine, where i tried to deploy just released Gerrit 2.6-rc1 by Shawn with my new and shiny gerrit-gitblit plugin, but somehow MessageFormat rendering failed: Caused by: java.lang.IllegalArgumentException: unknown format type at The problem was again that port 9418 already in use. Not sure if it is a bug in that line? Anyway, i disabled GitDaemon service in gitblit-gerrit, i think it doesn't make any sense in that context https://gerrit-review.googlesource.com/#/c/44263/ Thanks |
There are probably several settings that do not make sense for Gitblit-Gerrit. That is one of them. I would replace your existing gitlbit-gerrit.properties file with the current gitblit.properties from master, review all settings, and tweak the ones that should be changed. The exception you are reporting was fixed on master last week. |
Fist of all thank you for integrating JGut 3.0.0-SNAPSHOT! I wonder why line 163 in RepositoriesPage.java is failing to load welcome.mkd? It is in place in gerrit-gitblit.jar in root:
jar -tf gitblit.jar | grep -i welcome.mkd
welcome.mkd
Instead of welcome message i see this (from Gerit GitBlit Plugin):
Failed to read default message from welcome.mkd!
I am going to check the vanilla war and was trying to build it and it failed:
BUILD FAILED
/home/davido/projects/gitblit_src/build.xml:272: org.moxie.MoxieException: Unable to resolve: com/gitblit/GitServlet.class
com/gitblit/GitServlet.class could not be located on the classpath.
So i adjusted the path. Not sure if GitServlet.java should be moved?
Thanks
David