You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@yetus.apache.org by GitBox <gi...@apache.org> on 2020/02/06 19:19:42 UTC
[GitHub] [yetus] ndimiduk opened a new pull request #86: YETUS-934.
Precommit does not accept a caller's value of MAVEN_ARGS
ndimiduk opened a new pull request #86: YETUS-934. Precommit does not accept a caller's value of MAVEN_ARGS
URL: https://github.com/apache/yetus/pull/86
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [yetus] ndimiduk commented on a change in pull request #86:
YETUS-934. Precommit does not accept a caller's value of MAVEN_ARGS
Posted by GitBox <gi...@apache.org>.
ndimiduk commented on a change in pull request #86: YETUS-934. Precommit does not accept a caller's value of MAVEN_ARGS
URL: https://github.com/apache/yetus/pull/86#discussion_r376032958
##########
File path: precommit/src/main/shell/test-patch.d/maven.sh
##########
@@ -14,7 +14,9 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-declare -a MAVEN_ARGS
+# Accept a value of MAVEN_ARGS from the calling context
+# shellcheck disable=SC2206
Review comment:
Shellcheck tells me that I need to quote everything inside the parentheses, but I want `MAVEN_ARGS` to be interpreted as an array. If I quote it, it doesn't do the splitting I'm after.
```sh
$ x='hello there'
$ unset y
$ declare -a FOO=(${x:-})
$ set | grep FOO
FOO=([0]="hello" [1]="there")
_=FOO
$ declare -a FOO=("${x:-}")
$ set | grep FOO
FOO=([0]="hello there")
_=FOO
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [yetus] ndimiduk closed pull request #86: YETUS-934. Precommit does
not accept a caller's value of MAVEN_ARGS
Posted by GitBox <gi...@apache.org>.
ndimiduk closed pull request #86: YETUS-934. Precommit does not accept a caller's value of MAVEN_ARGS
URL: https://github.com/apache/yetus/pull/86
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [yetus] saintstack commented on issue #86: YETUS-934. Precommit
does not accept a caller's value of MAVEN_ARGS
Posted by GitBox <gi...@apache.org>.
saintstack commented on issue #86: YETUS-934. Precommit does not accept a caller's value of MAVEN_ARGS
URL: https://github.com/apache/yetus/pull/86#issuecomment-583502525
Let me try that @aw-was-here Thanks.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [yetus] aw-was-here commented on issue #86: YETUS-934. Precommit
does not accept a caller's value of MAVEN_ARGS
Posted by GitBox <gi...@apache.org>.
aw-was-here commented on issue #86: YETUS-934. Precommit does not accept a caller's value of MAVEN_ARGS
URL: https://github.com/apache/yetus/pull/86#issuecomment-583494485
> We're trying to add the `--threads ...` argument to the maven incantation. This is not for surefire's benefit, but for maven itself.
Oh, then modify:
```bash
extra="-DHBasePatchProcess"
```
to include it. Just be aware that not all plugins are thread-safe and can result in weird things happening.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [yetus] busbey commented on a change in pull request #86:
YETUS-934. Precommit does not accept a caller's value of MAVEN_ARGS
Posted by GitBox <gi...@apache.org>.
busbey commented on a change in pull request #86: YETUS-934. Precommit does not accept a caller's value of MAVEN_ARGS
URL: https://github.com/apache/yetus/pull/86#discussion_r376078706
##########
File path: precommit/src/main/shell/test-patch.d/maven.sh
##########
@@ -14,7 +14,9 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-declare -a MAVEN_ARGS
+# Accept a value of MAVEN_ARGS from the calling context
+# shellcheck disable=SC2206
Review comment:
were these two different attempts? I'd expect the first to fail and the second to succeed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [yetus] aw-was-here commented on issue #86: YETUS-934. Precommit
does not accept a caller's value of MAVEN_ARGS
Posted by GitBox <gi...@apache.org>.
aw-was-here commented on issue #86: YETUS-934. Precommit does not accept a caller's value of MAVEN_ARGS
URL: https://github.com/apache/yetus/pull/86#issuecomment-583480369
Never mind. I forgot that MAVEN_OPTS is for JVM options. Haha.
Since the goal here is to set options for surefire, then why not just add these to the chain in the unit control in the personality like the other maven command line options in the hbase personality?
```bash
if [[ ${testtype} = unit ]]; then
extra="${extra} -PrunAllTests"
# Inject the jenkins build-id for our surefire invocations
# Used by zombie detection stuff, even though we're not including that yet.
if [ -n "${BUILD_ID}" ]; then
extra="${extra} -Dbuild.id=${BUILD_ID}"
fi
fi
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [yetus] busbey commented on a change in pull request #86:
YETUS-934. Precommit does not accept a caller's value of MAVEN_ARGS
Posted by GitBox <gi...@apache.org>.
busbey commented on a change in pull request #86: YETUS-934. Precommit does not accept a caller's value of MAVEN_ARGS
URL: https://github.com/apache/yetus/pull/86#discussion_r376143358
##########
File path: precommit/src/main/shell/test-patch.d/maven.sh
##########
@@ -14,7 +14,9 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-declare -a MAVEN_ARGS
+# Accept a value of MAVEN_ARGS from the calling context
+# shellcheck disable=SC2206
Review comment:
Ugh. Hbase nightly saves the entire Yetus install in build artifacts but not the actual personality file used.
Lemme see what this looks like locally.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [yetus] aw-was-here commented on issue #86: YETUS-934. Precommit
does not accept a caller's value of MAVEN_ARGS
Posted by GitBox <gi...@apache.org>.
aw-was-here commented on issue #86: YETUS-934. Precommit does not accept a caller's value of MAVEN_ARGS
URL: https://github.com/apache/yetus/pull/86#issuecomment-583341975
Reading through this and HBASE-23779, it feels like there is a misunderstanding of two different environment variables happening.
MAVEN_OPTS is where Apache Maven expects external settings to come from.
MAVEN_ARGS is how Apache Yetus sets its internal Apache Maven arguments.
In almost all circumstances, MAVEN_OPTS is what users should be setting outside of the codebase to control what Apache Maven does when precommit uses it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [yetus] saintstack commented on a change in pull request #86:
YETUS-934. Precommit does not accept a caller's value of MAVEN_ARGS
Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #86: YETUS-934. Precommit does not accept a caller's value of MAVEN_ARGS
URL: https://github.com/apache/yetus/pull/86#discussion_r376066133
##########
File path: precommit/src/main/shell/test-patch.d/maven.sh
##########
@@ -14,7 +14,9 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-declare -a MAVEN_ARGS
+# Accept a value of MAVEN_ARGS from the calling context
+# shellcheck disable=SC2206
Review comment:
This is what I pushed last night into hbase-personality... it don't work either.
- export MAVEN_ARGS="-T0.5C ${MAVEN_ARGS}"
+ MAVEN_ARGS=("-T0.5C" "${MAVEN_ARGS[@]}")
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [yetus] busbey commented on issue #86: YETUS-934. Precommit does
not accept a caller's value of MAVEN_ARGS
Posted by GitBox <gi...@apache.org>.
busbey commented on issue #86: YETUS-934. Precommit does not accept a caller's value of MAVEN_ARGS
URL: https://github.com/apache/yetus/pull/86#issuecomment-583485739
let me try that.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [yetus] ndimiduk commented on a change in pull request #86:
YETUS-934. Precommit does not accept a caller's value of MAVEN_ARGS
Posted by GitBox <gi...@apache.org>.
ndimiduk commented on a change in pull request #86: YETUS-934. Precommit does not accept a caller's value of MAVEN_ARGS
URL: https://github.com/apache/yetus/pull/86#discussion_r376087885
##########
File path: precommit/src/main/shell/test-patch.d/maven.sh
##########
@@ -14,7 +14,9 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-declare -a MAVEN_ARGS
+# Accept a value of MAVEN_ARGS from the calling context
+# shellcheck disable=SC2206
Review comment:
> Why are we trying to get an initial MAVEN_ARGS from the environment? the use of personality in e.g. HBASE-23779 should be adding to an array we declare here, not creating a new environment variable that we need to parse.
@busbey you saying we have the order of operations wrong? Personality appends to the base plugin?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [yetus] saintstack commented on a change in pull request #86:
YETUS-934. Precommit does not accept a caller's value of MAVEN_ARGS
Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #86: YETUS-934. Precommit does not accept a caller's value of MAVEN_ARGS
URL: https://github.com/apache/yetus/pull/86#discussion_r376137412
##########
File path: precommit/src/main/shell/test-patch.d/maven.sh
##########
@@ -14,7 +14,9 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-declare -a MAVEN_ARGS
+# Accept a value of MAVEN_ARGS from the calling context
+# shellcheck disable=SC2206
Review comment:
Issue HBASE-23779 has the jobs I ran after pushing 'fixes'. hopefully: https://builds.apache.org/view/H-L/view/HBase/job/HBase%20Nightly/job/branch-2/2452/ and then 2454 is the one that ran w/ second attempt.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [yetus] busbey commented on a change in pull request #86:
YETUS-934. Precommit does not accept a caller's value of MAVEN_ARGS
Posted by GitBox <gi...@apache.org>.
busbey commented on a change in pull request #86: YETUS-934. Precommit does not accept a caller's value of MAVEN_ARGS
URL: https://github.com/apache/yetus/pull/86#discussion_r376062965
##########
File path: precommit/src/main/shell/test-patch.d/maven.sh
##########
@@ -14,7 +14,9 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-declare -a MAVEN_ARGS
+# Accept a value of MAVEN_ARGS from the calling context
+# shellcheck disable=SC2206
Review comment:
Why are we trying to get an initial MAVEN_ARGS from the environment? the use of personality in e.g. HBASE-23779 should be adding to an array we declare here, not creating a new environment variable that we need to parse.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [yetus] ndimiduk commented on a change in pull request #86:
YETUS-934. Precommit does not accept a caller's value of MAVEN_ARGS
Posted by GitBox <gi...@apache.org>.
ndimiduk commented on a change in pull request #86: YETUS-934. Precommit does not accept a caller's value of MAVEN_ARGS
URL: https://github.com/apache/yetus/pull/86#discussion_r376086476
##########
File path: precommit/src/main/shell/test-patch.d/maven.sh
##########
@@ -14,7 +14,9 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-declare -a MAVEN_ARGS
+# Accept a value of MAVEN_ARGS from the calling context
+# shellcheck disable=SC2206
Review comment:
Stack's addendum was included in https://builds.apache.org/job/HBase%20Nightly/job/master/1622/
I don't see his tweaked MAVEN_ARGS in any of the build invocation logs, for example, https://builds.apache.org/job/HBase%20Nightly/job/master/1622/artifact/output-jdk8-hadoop2/patch-compile-root.txt
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [yetus] busbey commented on a change in pull request #86:
YETUS-934. Precommit does not accept a caller's value of MAVEN_ARGS
Posted by GitBox <gi...@apache.org>.
busbey commented on a change in pull request #86: YETUS-934. Precommit does not accept a caller's value of MAVEN_ARGS
URL: https://github.com/apache/yetus/pull/86#discussion_r376144102
##########
File path: precommit/src/main/shell/test-patch.d/maven.sh
##########
@@ -14,7 +14,9 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-declare -a MAVEN_ARGS
+# Accept a value of MAVEN_ARGS from the calling context
+# shellcheck disable=SC2206
Review comment:
> @busbey you saying we have the order of operations wrong? Personality appends to the base plugin?
Yes. The docs say `personality_globals` is done after importing plugins.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [yetus] ndimiduk commented on issue #86: YETUS-934. Precommit does
not accept a caller's value of MAVEN_ARGS
Posted by GitBox <gi...@apache.org>.
ndimiduk commented on issue #86: YETUS-934. Precommit does not accept a caller's value of MAVEN_ARGS
URL: https://github.com/apache/yetus/pull/86#issuecomment-583488943
> Since the goal here is to set options for surefire
Heya @aw-was-here. This is not quite correct. We're trying to add the `--threads ...` argument to the maven incantation. This is not for surefire's benefit, but for maven itself. Either way, we're just adding another argument to the command, so maybe it doesn't matter either way? I don't know if maven expects its optional arguments to come ahead of the positional arguments.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [yetus] saintstack commented on a change in pull request #86:
YETUS-934. Precommit does not accept a caller's value of MAVEN_ARGS
Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #86: YETUS-934. Precommit does not accept a caller's value of MAVEN_ARGS
URL: https://github.com/apache/yetus/pull/86#discussion_r376154056
##########
File path: precommit/src/main/shell/test-patch.d/maven.sh
##########
@@ -14,7 +14,9 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-declare -a MAVEN_ARGS
+# Accept a value of MAVEN_ARGS from the calling context
+# shellcheck disable=SC2206
Review comment:
Thanks @busbey (and @ndimiduk )
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [yetus] ndimiduk commented on issue #86: YETUS-934. Precommit does
not accept a caller's value of MAVEN_ARGS
Posted by GitBox <gi...@apache.org>.
ndimiduk commented on issue #86: YETUS-934. Precommit does not accept a caller's value of MAVEN_ARGS
URL: https://github.com/apache/yetus/pull/86#issuecomment-583623009
To circle back here, looks adding to the `extras` in the HBase personality achieved @saintstack 's objective. Let's shut this one down.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [yetus] aw-was-here edited a comment on issue #86: YETUS-934.
Precommit does not accept a caller's value of MAVEN_ARGS
Posted by GitBox <gi...@apache.org>.
aw-was-here edited a comment on issue #86: YETUS-934. Precommit does not accept a caller's value of MAVEN_ARGS
URL: https://github.com/apache/yetus/pull/86#issuecomment-583341975
Reading through this and HBASE-23779, it feels like there is a misunderstanding of two different environment variables happening.
MAVEN_OPTS is where Apache Maven expects external settings to come from.
MAVEN_ARGS is how Apache Yetus sets its internal Apache Maven arguments.
In almost all circumstances, MAVEN_OPTS is what users should be setting outside of the maven.sh codebase to control what Apache Maven does when precommit uses it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services