Skip to content
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.

Sign up for GitHub

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

Use standard sed syntax for translating filenames #4072

Merged
merged 3 commits into from
Dec 3, 2024
Merged

Use standard sed syntax for translating filenames #4072

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
47326da
Use standard sed syntax for translating filenames
sxa Dec 2, 2024
290bcb3
Comment
sxa Dec 3, 2024
14d9387
Merge branch 'master' into solarisfixes
sxa Dec 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion sbin/build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2106,7 +2106,10 @@ getTargetFileNameForComponent() {
echo "${target_file_name}" | sed "s/-jdk/-${component}/"
else
# if no pattern is found, append the component name right before the extension.
echo "${target_file_name}" | sed -r "s/(.+)(\.tar\.gz|\.zip)/\1-${component}\2/"
# Stopped using -r here and split this in 2 as -r not a standard sed argument
echo "${target_file_name}" | sed \
Copy link
Contributor

Choose a reason for hiding this comment

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

Worth adding a comment about using Solaris friendly sed? I've seen a few cases where this sort of thing gets reverted by Linux focused folks :-)

Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't shell check able to to check posix? But I guess that is syntax only and not sed/grep and others...

Copy link
Member Author

@sxa sxa Dec 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Worth adding a comment about using Solaris friendly sed? I've seen a few cases where this sort of thing gets reverted by Linux focused folks :-)

I have a view on that ... If we think we really need to tell people explictly "Don't change this to a non-standard syntax" I can add it (such things aren't just for Solaris)... however I feel adding such comments everywhere would be a job for life ;-) "Linux folks" need to be aware that this isn't just a Linux project when copying stuff from stackoverflow/ChatGPT etc. ;-)

Isn't shell check able to to check posix? But I guess that is syntax only and not sed/grep and others...

I suspect so yeah. There have also been cases where we have used if blocks to explicitly check for and use non-standard versions, so they'd have to be skipped.

-e "s/\(.*\)\(\.tar\.gz\)/\1-$component\2/" \
-e "s/\(.*\)\(\.zip\)/\1-${component}\2/"
fi
}

Expand Down
Loading