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

Dynamically Detect Nginx Parameters and Install Additional Packages for Successful Compilation with Image #29

Conversation

takashi-osako-compass
Copy link

@takashi-osako-compass takashi-osako-compass commented Jun 16, 2023

This pull request aims to enhance the installation process of the module by dynamically reading the parameters used to build Nginx if it is already installed. By doing so, we ensure that the module is compiled with the same parameters, promoting compatibility and consistency.

To achieve this, the following changes have been made:

  1. Introduce a parameter detection mechanism: The installation script now checks if Nginx is installed and retrieves its build parameters dynamically. This ensures that the module is compiled using the same options, flags, and configuration as the existing Nginx installation.

  2. Add compatibility with the Buster image: In order to compile successfully with the image, additional packages have been included as dependencies. These packages are now automatically installed during the compilation process, guaranteeing a smooth and error-free build.

@takashi-osako-compass takashi-osako-compass changed the title Dynamically Detect Nginx Parameters and Install Additional Packages for Successful Compilation with Buster Image Dynamically Detect Nginx Parameters and Install Additional Packages for Successful Compilation with Image Jun 16, 2023
@dgoffredo
Copy link
Contributor

dgoffredo commented Jun 16, 2023

Thank you for proposing these changes, @takashi-osako-compass!

I'm glad to see that you got it working.

My teammate and I had been discussing how we ought to support build modes where the ./configure arguments differ from what we currently use. We imagined a "build telemetry script" that would detect an nginx installation and collect information about it, like you have done here. Additionally, we wanted to include information about the host system, standard c library, etc.

I won't merge these changes exactly as you've proposed them, because there is overlap with the solution that we will come up with. Also, there are a few small details that I would change. I'll leave this pull request open until this "build telemetry script" is written.

Hopefully by the next release, you will be able to build the module within your image without having to maintain a patch like this.

Thanks again for the contribution.

@takashi-osako-compass
Copy link
Author

Thank you for your feedback! I'm thrilled to hear fix is on the way!

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