Skip to content

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

Merged
okumin merged 1 commit into
apache:masterfrom
bond-:feature/fix-shell-scripts-in-packaging
May 10, 2025
Merged

HIVE-28937: Static analysis issues fixed in shell scripts#5797
okumin merged 1 commit into
apache:masterfrom
bond-:feature/fix-shell-scripts-in-packaging

Conversation

@bond-

@bond- bond- commented Apr 30, 2025

Copy link
Copy Markdown
Contributor

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.

@sonarqubecloud

sonarqubecloud Bot commented May 5, 2025

Copy link
Copy Markdown

@okumin okumin left a comment

Copy link
Copy Markdown
Contributor

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-

bond- commented May 9, 2025

Copy link
Copy Markdown
Contributor Author

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

@okumin

okumin commented May 10, 2025

Copy link
Copy Markdown
Contributor

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
@okumin

okumin commented May 10, 2025

Copy link
Copy Markdown
Contributor

@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-

bond- commented May 11, 2025

Copy link
Copy Markdown
Contributor Author

@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

okumin commented May 12, 2025

Copy link
Copy Markdown
Contributor

@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