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

Fixed installation of dev_headers and postgis on RedHat #200

Closed
wants to merge 5 commits into from

Conversation

mjuenema
Copy link

@mjuenema mjuenema commented Sep 21, 2016

This fixes the problem that there where o steps to install dev_headers and postgis on RedHat. This should also fix issue #194.

Markus

with_items:
- "postgis2_{{postgresql_version_terse}}"
- "postgis2_{{postgresql_version_terse}}-client"
- "postgis2_{{postgresql_version_terse}}-devel"
Copy link
Contributor

Choose a reason for hiding this comment

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

It's really need for production server install devel and docs?

Copy link
Author

Choose a reason for hiding this comment

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

Not necessarily. This could be made conditional on postgresql_ext_install_dev_headers, assuming that if you want the development files for PostgreSQL itself you also want the PostGIS development files. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

PostgreSQL dev headers not corellate with postgis extension. I think, installation of dev packages should be manually. For production use its not necessary.

Copy link
Author

Choose a reason for hiding this comment

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

I agree that the dev packages are not required for production use. On the other hand the role already has the option to install dev packages (headers) if one choses so. So my thought was that if you install the PostgreSQL dev packages and chose to select PostGIS there is a good chance that the user wants the PostGIS dev package, too. Having to install it manually defeats the purpose of automation. For example, I am using the role together with Vagrant to create a PostGIS development server.

Copy link
Member

@otakup0pe otakup0pe Sep 28, 2016

Choose a reason for hiding this comment

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

If the dev headers for postgresql aren't needed when installing the postgis basics then it shouldn't get conflated. I'm guessing this is how the https://github.com/ANXS/utilities role came about originally heh.

with_items:
- "postgis2_{{postgresql_version_terse}}-devel"
- "postgis2_{{postgresql_version_terse}}-docs"
when: (ansible_os_family == "RedHat") and (postgresql_ext_install_dev_headers)
Copy link
Contributor

Choose a reason for hiding this comment

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

when: ansible_os_family == "RedHat" and {{ postgresql_ext_install_dev_headers | default(false) }}

Copy link
Contributor

Choose a reason for hiding this comment

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

Or add this variable into defaults file

Copy link
Author

Choose a reason for hiding this comment

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

I am happy with that.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the dev_headers was fixed by #327 , but the PostGIS dev headers aren't done. It may be best to add it to #310?

Copy link
Contributor

Choose a reason for hiding this comment

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

sure you can do that

@taybin
Copy link

taybin commented Dec 29, 2016

Can this get merged in now?

@mjuenema
Copy link
Author

mjuenema commented Jan 2, 2017

@taybin Fine with me.

@aoyawale aoyawale closed this Apr 11, 2018
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.

6 participants