Alon Bar-Lev has posted comments on this change.

Change subject: webadmin,userportal: Improve GWT build
......................................................................


Patch Set 6:

(5 comments)

http://gerrit.ovirt.org/#/c/36739/6/Makefile
File Makefile:

Line 34: EXTRA_BUILD_FLAGS=
Line 35: BUILD_VALIDATION=1
Line 36: BUILD_ENV_VALIDATION=1
Line 37: BUILD_JAVA_OPTS_MAVEN=-XX:MaxPermSize=512M
Line 38: BUILD_JAVA_OPTS_GWT=-Xms1024M -Xmx8192M -XX:PermSize=512M 
-XX:MaxPermSize=1024M
> > what happened to common aka BUILD_JAVA_OPTS_COMMON you wanted?
it enables to override based on environment. if you do not do so, we need to 
alter spec to enable rpm macro to receive these settings using -Dxxx
Line 39: DEV_REBUILD=1
Line 40: DEV_BUILD_GWT_DRAFT=0
Line 41: DEV_EXTRA_BUILD_FLAGS=
Line 42: DEV_EXTRA_BUILD_FLAGS_GWT_DEFAULTS=


Line 117: ifneq ($(BUILD_DEV),0)
Line 118: BUILD_FLAGS:=$(BUILD_FLAGS) $(DEV_BUILD_FLAGS)
Line 119: endif
Line 120: BUILD_FLAGS:=$(BUILD_FLAGS) $(EXTRA_BUILD_FLAGS)
Line 121: BUILD_FLAGS:=$(BUILD_FLAGS) -D gwt.jvmArgs="$(BUILD_JAVA_OPTS_GWT)"
> Above change just adds Maven gwt.jvmArgs property override to         
> BUILD_FLAGS.
$(EXTRA_BUILD_FLAGS) was removed
Line 122: 
Line 123: PYTHON_SYS_DIR:=$(shell $(PYTHON) -c "from distutils.sysconfig import 
get_python_lib as f;print(f())")
Line 124: OUTPUT_RPMBUILD=$(shell pwd -P)/tmp.rpmbuild
Line 125: OUTPUT_DIR=output


Line 229:       chmod a+x packaging/bin/ovirt-engine-log-setup-event.sh
Line 230: 
Line 231: # support force run of maven
Line 232: maven:
Line 233:       export MAVEN_OPTS="${MAVEN_OPTS} ${BUILD_JAVA_OPTS_MAVEN}"
> OK, I'll make adjustments as suggested.
> But we did it before, I didn't know we don't want to do this now.

yes, but if we touch this... better to make it right.
Line 234:       $(MVN) \
Line 235:               $(BUILD_FLAGS) \
Line 236:               $(BUILD_TARGET)
Line 237:       touch "$(BUILD_FILE)"


http://gerrit.ovirt.org/#/c/36739/6/frontend/webadmin/modules/pom.xml
File frontend/webadmin/modules/pom.xml:

Line 49:       
-Dgwt.jjs.permutationWorkerFactory=com.google.gwt.dev.ThreadedPermutationWorkerFactory
 \
Line 50:       -Dgwt.jjs.maxThreads=4 \
Line 51:       -Djava.io.tmpdir="${project.build.directory}/tmp" \
Line 52:       -Djava.util.prefs.systemRoot="${project.build.directory}/tmp" \
Line 53:       -Djava.util.prefs.userRoot="${project.build.directory}/tmp" \
> Because it turns out that ".java" dir is created anyway, so before it was:
oh... are you sure? it should be the .java location... strange... but if .java 
is created then we are ok.
Line 54:       ${gwt.jvmArgs}
Line 55:     </gwt-plugin.extraJvmArgs>
Line 56:     <!-- Custom JVM arguments for GWT compiler and Dev Mode, empty by 
default -->
Line 57:     <gwt.jvmArgs/>


Line 53:       -Djava.util.prefs.userRoot="${project.build.directory}/tmp" \
Line 54:       ${gwt.jvmArgs}
Line 55:     </gwt-plugin.extraJvmArgs>
Line 56:     <!-- Custom JVM arguments for GWT compiler and Dev Mode, empty by 
default -->
Line 57:     <gwt.jvmArgs/>
> OK, so in Makefile I'll need to add check if BUILD_JAVA_OPTS_GWT is not emp
sure, or... add the defaults here and ignore external flags if empty, whatever 
easier.
Line 58:     <!-- Control target browsers for GWT compilation, maps to 
'user.agent' deferred binding property in *.gwt.xml -->
Line 59:     <!-- By default, compile for Firefox browser only, use 
'all-user-agents' profile to compile for all browsers -->
Line 60:     <gwt.userAgent>gecko1_8</gwt.userAgent>
Line 61:     <!-- Control target locales for GWT compilation, maps to 'locale' 
deferred binding property in *.gwt.xml -->


-- 
To view, visit http://gerrit.ovirt.org/36739
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaeb92d69f2ba38746559df3e44f34a61fd880908
Gerrit-PatchSet: 6
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Vojtech Szocs <[email protected]>
Gerrit-Reviewer: Alexander Wels <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Barak Korren <[email protected]>
Gerrit-Reviewer: David Caro <[email protected]>
Gerrit-Reviewer: Einav Cohen <[email protected]>
Gerrit-Reviewer: Eyal Edri <[email protected]>
Gerrit-Reviewer: Lior Vernia <[email protected]>
Gerrit-Reviewer: Sandro Bonazzola <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to