Alon Bar-Lev has posted comments on this change. Change subject: webadmin,userportal: Improve GWT build ......................................................................
Patch Set 6: (6 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? don't you want to control this via environment? if you do replace the = with ?= 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)" why have you removed the EXTRA_BUILD_FLAGS? 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}" please rework this into: MAVEN_OPTS="${MAVEN_OPTS} $(BUILD_JAVA_OPTS_MAVEN)" \ $(MVN) we do not need to export this. also notice the () Line 234: $(MVN) \ Line 235: $(BUILD_FLAGS) \ Line 236: $(BUILD_TARGET) Line 237: touch "$(BUILD_FILE)" Line 498: ln -sf "$(DATA_DIR)/conf/osinfo-defaults.properties" "$(DESTDIR)$(PKG_SYSCONF_DIR)/osinfo.conf.d/00-defaults.properties" Line 499: Line 500: gwt-debug: Line 501: [ -n "$(DEBUG_MODULE)" ] || ( echo "Please specify DEBUG_MODULE" && false ) Line 502: $(MVN) -pl frontend/webadmin/modules/$(DEBUG_MODULE) \ please keep the quotes not sure I see change in this hank that related to the flags, better to have this in own patch, although it is not actually an improvement as it has the same effect, no? :) Line 503: $(DEV_EXTRA_BUILD_FLAGS_GWT_DEFAULTS) \ Line 504: $(DEV_EXTRA_BUILD_FLAGS) \ Line 505: -Dgwt.noserver=true \ Line 506: -Pgwtdev,gwt-admin,gwt-user \ 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" \ why have you removed the .java for properties root? 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/> I do expect sane default for jvmArgs for people which do not run maven via the make. 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 <vsz...@redhat.com> Gerrit-Reviewer: Alexander Wels <aw...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Barak Korren <bkor...@redhat.com> Gerrit-Reviewer: David Caro <dcaro...@redhat.com> Gerrit-Reviewer: Einav Cohen <eco...@redhat.com> Gerrit-Reviewer: Eyal Edri <ee...@redhat.com> Gerrit-Reviewer: Lior Vernia <lver...@redhat.com> Gerrit-Reviewer: Sandro Bonazzola <sbona...@redhat.com> Gerrit-Reviewer: Vojtech Szocs <vsz...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
