Skip to content

HIVE-28937: Static analysis issues fixed in shell scripts #5797

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

Merged
merged 1 commit into from
May 10, 2025

Conversation

bond-
Copy link
Contributor

@bond- bond- commented Apr 30, 2025

What changes were proposed in this pull request?

This pull request addresses several static analysis issues identified in the shell scripts located in the packaging folder. The issues were detected using ShellCheck and include:

  1. SC2086: Double quote to prevent globbing and word splitting. Link to SC2086
  2. SC2125: Brace expansion is not supported in POSIX sh. Link to SC2125
  3. SC2223: This default assignment may cause problems when concatenating strings. Link to SC2223

Why are the changes needed?

These changes are necessary to ensure that the shell scripts in the packaging folder adhere to best practices and avoid potential issues related to globbing, word splitting, brace expansion, and string concatenation. Addressing these issues will improve the reliability and maintainability of the scripts.

Does this PR introduce any user-facing change?

No, this PR does not introduce any user-facing changes. It solely focuses on improving the internal shell scripts.

How was this patch tested?

The patch was tested by running ShellCheck on the modified shell scripts to ensure that the identified issues were resolved. Additionally, the scripts were manually tested to verify that they function correctly after the changes.

Copy link

sonarqubecloud bot commented May 5, 2025

Copy link
Contributor

@okumin okumin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested it in the following way.

Build a snapshot Hive

mvn clean package -Dmaven.javadoc.skip=true -DskipTests -Pdist

Build an image

./packaging/src/docker/build.sh

Test HS2

docker run -it --rm --env SERVICE_NAME=hiveserver2 --name hive apache/hive:4.1.0-SNAPSHOT

@bond-
Copy link
Contributor Author

bond- commented May 9, 2025

@okumin thanks, who is going to be merging this PR?

@okumin
Copy link
Contributor

okumin commented May 10, 2025

I'll ignore the CI failure because this PR is very unrelated to our unit or integration tests

@okumin okumin merged commit acda3d0 into apache:master May 10, 2025
3 of 4 checks passed
@okumin
Copy link
Contributor

okumin commented May 10, 2025

@bond- Merged. Thanks for your contributions! It looks like you're trying to create a minimal Docker image to expose HMS. I personally think it is an important step to have production-grade images of Hive. I'm willing to help you.
https://docs.google.com/document/d/1tKFmsjYeGlMQjvJ7QQDNcS5wvqrcHRvsDjbJlcHb7Gk/edit?tab=t.0

Thanks.

@bond-
Copy link
Contributor Author

bond- commented May 11, 2025

@okumin sure more the better, let me know how you would like to collaborate? Slack?

@bond- bond- deleted the feature/fix-shell-scripts-in-packaging branch May 11, 2025 02:28
@okumin
Copy link
Contributor

okumin commented May 12, 2025

@bond-
The Apache Way requires us to make any decisions on the mailing list. I believe you can also use Slack to gather some information, e.g., by questioning me about my opinion or brainstorming it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants