-
Notifications
You must be signed in to change notification settings - Fork 52
HBASE-29465 [hbase-thirdparty] Update yetus to a newer version #144
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
Conversation
|
Based off apache/hbase@17ce7c3 and apache/hbase@4c83128 |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
🎊 +1 overall
This message was automatically generated. |
|
Ok so now the code is building but the tests run are not as expected! Do we need to provide test filter with new yetus.? |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
Ah now I understand! It was working good all this while, since we did not have any change in code hence build did not run javac, etc. Introduced some changes in pom.xml to trigger those. |
|
💔 -1 overall
This message was automatically generated. |
|
xmllint is fixed but other maven stuff failing now 🤦 |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
🎊 +1 overall
This message was automatically generated. |
| # Try multiple possible locations for the toolchains file | ||
| TOOLCHAIN_LOCATIONS=( | ||
| "${BASEDIR}/dev-support/toolchains-jenkins.xml" | ||
| "$(pwd)/dev-support/toolchains-jenkins.xml" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
may be we do not need to try for different dirs as it should found in 1st dir itself? but why not, not a problem too? WDYT?
| TOOLCHAIN="${BASEDIR}/dev-support/toolchains-jenkins.xml" | ||
| if [ -f "$TOOLCHAIN" ]; then | ||
|
|
||
| echo "Maven wrapper called with args: $@" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think its good to keep the change as this improves logging and error handling. without this the out files were empty and swallowing errors making difficult to debug
|
Now the issue is fixed, problem was usage of exec and no error handling, hence logs were empty. Now we have better error handling and proper logging in wrapper code. Also added a dummy commit in each module to trigger all possible build combinations. Will revert before commit! |
This comment was marked as outdated.
This comment was marked as outdated.
| done | ||
| echo "Proceeding without toolchains configuration..." | ||
| echo "Executing: ${MAVEN_HOME}/bin/mvn-original $@" | ||
| "${MAVEN_HOME}/bin/mvn-original" "$@" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this fallback at least builds code, should not happen again though, but maybe with some yetus upgrade or dockerfile change
|
💔 -1 overall
This message was automatically generated. |
|
ok looks good, blanks is due to the dummy patch |
|
reverted dummy patch, fixed some typos, changed message to error from warning |
|
(!) A patch to the testing environment has been detected. |
|
🎊 +1 overall
This message was automatically generated. |
0.15.0, making required changes in sync with hbase main repo