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.

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

Simplify .deb and .rpm LSC package generation #112

Merged
merged 7 commits into from
Jun 29, 2018

Conversation

timopollmeier
Copy link
Member

The gvm-lsc-rpm-creator script is replaced with a simpler script that
does not include makeself and a new script is used to generate Debian
packages directly with dpkg instead of using alien to convert the RPM.
Because of this, checks for alien are removed and the INSTALL doc is
updated accordingly.

The gvm-lsc-rpm-creator script is replaced with a simpler script that
does not include makeself and a new script is used to generate Debian
packages directly with dpkg instead of using alien to convert the RPM.
Because of this, checks for alien are removed and the INSTALL doc is
updated accordingly.
@timopollmeier timopollmeier requested a review from a team May 18, 2018 09:16
@timopollmeier timopollmeier self-assigned this May 18, 2018

# Create .ssh directory
mkdir -p "${SSH_DATA_DIR}"
if [ 0 -ne "$?" ]
Copy link
Member

Choose a reason for hiding this comment

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

Consider using if ! mkdir -p "${SSH_DATA_DIR}" here

mkdir -p "${DOC_DATA_DIR}"

# Create Changelog
cd "${DOC_DATA_DIR}"
Copy link
Member

Choose a reason for hiding this comment

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

Consider using 'cd ... || exit' to not simply continue on cd failures

cd "${DOC_DATA_DIR}"
CHANGELOG_FILE="${DOC_DATA_DIR}/changelog.Debian"
echo "${PACKAGE_NAME} (${PACKAGE_VERSION}) experimental; urgency=low" > ${CHANGELOG_FILE}
echo "" >> ${CHANGELOG_FILE}
Copy link
Member

Choose a reason for hiding this comment

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

Consider using { cmd1; cmd2; } >> file instead of individual redirects.

@timopollmeier timopollmeier added the work in progress This pull request should not be merged yet, more commits are expected label May 25, 2018
In the post-installation of the generated packages, change the owner of
the whole home directory, not just the .ssh subdirectory.
Also change the owner group to the given username.
File variables are now used with quotes to prevent globbing.
Using 'if [ 0 -ne "$?" ]' is now avoided.
Blocks of commands generating content like multiple echos are run in a
subshell so only one redirection is needed per file.
@timopollmeier timopollmeier removed the work in progress This pull request should not be merged yet, more commits are expected label Jun 26, 2018
@timopollmeier
Copy link
Member Author

@wiegandm: I've changed the scripts according to most of those hints but I'm not sure about the one about adding || exit to all cd commands.
I wonder if we should add "set -e" to abort on any error or add || exit 1 to all commands.

@wiegandm
Copy link
Member

I wonder if we should add "set -e" to abort on any error or add || exit 1 to all commands.

Good point! I think in the interest of both readability and consistent error handling, using set -e is indeed a good idea. I would suggest combining it with a trap handling the ERR signal. In this trap, you could do clean-ups (if necessary) and report/log that your script is exiting abnormally.

The LSC package generator scripts now exit on any error, sending an
error message to stderr.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants