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

Simpler build scripts #1964

Merged
merged 8 commits into from
Mar 20, 2019
Merged

Simpler build scripts #1964

merged 8 commits into from
Mar 20, 2019

Conversation

alexjurkiewicz
Copy link
Contributor

This is a pretty invasive change to the build system, but I didn't see a good way to split it up.

There are two major improvements:

  1. Single build script for each package type (rpm, deb), rather than three
  2. Simplified and hardened the build script

There are also a few minor fixes I found along the way:

  1. Add error logging for make install and make uninstall on unsupported OS (eg macOS)
  2. Fix RPM build warning
  3. Delete extra directory during deb builder pre-cleanup for consistency with rpm builder

Write to /var/log/proxysql.log by default, and add a logrotate script
which manages this path.

Unfortunately, the logrotate script can't use `PROXYSQL FLUSH LOGS;` to
tell ProxySQL to rotate file handles*, so we use copytruncate instead.
It would be nice if ProxySQL could rotate file handles following a
signal in future...

* For two reasons:
1. There's no safe / standard way to find admin credentials
2. mysql(1) client may not be installed
This adds an error message to `make install/uninstall` on "unsupported"
distros. The behaviour is the same as before (do nothing, successfully).
The build scripts for all three packaging targets are identical except
for their Make targets. Unify them, and pass in a new environment
variable PROXYSQL_BUILD_TYPE to define the build type we want.

The build scripts are updated to automatically exit if any command fails
(`set -e`), so the "&& \" pattern is no longer required. We also
`set -u` to protect against any variable being accidentally unset.
We don't need the `&& \` pattern any more now that `set -e` is defined
at the top of the script. Also apply several defensive fixes suggested
by the Shellcheck linter (https://github.com/koalaman/shellcheck).
@alexjurkiewicz
Copy link
Contributor Author

Also, I based this on top of #1963, so it includes that change

@pondix
Copy link
Contributor

pondix commented Mar 20, 2019

Automated message: PR pending admin approval for build testing

@pondix
Copy link
Contributor

pondix commented Mar 20, 2019

This is really great @alexjurkiewicz thank you very much! It was on the todo list :) and it really does simplify making changes, I'm going to merge it into 2.0.3 and build the new release with the upgraded scripts.

@pondix pondix merged commit 39a0c9c into sysown:v2.0.3 Mar 20, 2019
@pondix
Copy link
Contributor

pondix commented Mar 20, 2019

There is an issue with the script @alexjurkiewicz

centos67_build_1 | /opt/entrypoint/entrypoint.bash: line 25: build_target[@]: unbound variable

pondix pushed a commit that referenced this pull request Mar 20, 2019
pondix pushed a commit that referenced this pull request Mar 20, 2019
@alexjurkiewicz
Copy link
Contributor Author

Sorry! I built the latest rhel/Deb scripts to test. I will investigate today.

@pondix
Copy link
Contributor

pondix commented Mar 20, 2019

No problem, please make a PR against 2.0.4 since 2.0.3 is now released :) thanks again @alexjurkiewicz for your contribution!

@alexjurkiewicz
Copy link
Contributor Author

What branch name should I use? There isn't a v2.0.4 branch

@pondix
Copy link
Contributor

pondix commented Mar 21, 2019

hi @alexjurkiewicz - I've pushed branch + tag for 2.0.4

pondix pushed a commit that referenced this pull request Mar 26, 2019
The build scripts for all three packaging targets are identical except
for their Make targets. Unify them, and pass in a new environment
variable PROXYSQL_BUILD_TYPE to define the build type we want.

The build scripts are updated to automatically exit if any command fails
(`set -e`), so the "&& \" pattern is no longer required. We also
`set -u` to protect against any variable being accidentally unset.

We don't need the `&& \` pattern any more now that `set -e` is defined
at the top of the script. Also apply several defensive fixes suggested
by the Shellcheck linter (https://github.com/koalaman/shellcheck).

Other small changes:
* fix: Add missing directory to deb script cleanup
* fix: cosmetics
* fix: Don't define config file twice in RPM spec
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