A start with splitting map.jinja and php works in focal (Ubuntu 20.04) #214

waynegemmell · 2020-06-08T20:37:32Z

PR progress checklist (to be filled in by reviewers)

Changes to documentation are appropriate (or tick if not required)
Changes to tests are appropriate (or tick if not required)
Reviews completed

What type of PR is this?

Primary type

[build] Changes related to the build system
[chore] Changes to the build process or auxiliary tools and libraries such as documentation generation
[ci] Changes to the continuous integration configuration
[ x ] [feat] A new feature
[fix] A bug fix
[perf] A code change that improves performance
[refactor] A code change that neither fixes a bug nor adds a feature
[revert] A change used to revert a previous commit
[style] Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc.)

Does this PR introduce a `BREAKING CHANGE`?

No.

Describe the changes you're proposing

I wanted to install Focal packages and allow for sensible defaults for PHP versions. PHP FPM was also not honouring the php version when creating it's files. I started with separating the map.jinja file but I don't really know how to finish that job. I got far enough that the libraries now install with the correct version for the relevant Ubuntu distribution by default.

With this change now most packages install without specifying a version. php.cli is an exception. It would work if the php object was merged instead of being overwritten. I couldn't work that out.

Pillar / config required to test the proposed changes

Nothing

pull-assistant · 2020-06-08T20:37:35Z

Best reviewed: commit by commit

Optimal code review plan

Added osfamilymap, osmap and other defaults

Installs packages in focal.

Powered by Pull Assistant. Last update 817c54b ... 11fb54e. Read the comment docs.

danny-smit · 2020-10-14T16:19:40Z

This is actually a great improvement and could resolve a bugreport that I wanted to write.
Is this almost done? What needs to be done to get this merged?

danny-smit · 2020-10-14T16:22:47Z

php/osfingermap.yaml

+# osfingermap: {}
+---
+# os: Debian
+Debian-10: {}


Suggested change

Debian-10: {}

Debian-10:

pillar_php_version: "7.3"

waynegemmell · 2020-10-14T18:45:53Z

I use it already. I'm sure some improvements could be made though, but it works for me atm.

…

On Wed, Oct 14, 2020 at 6:23 PM Danny Smit ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In php/osfingermap.yaml <#214 (comment)> : > @@ -0,0 +1,48 @@ +# -*- coding: utf-8 -*- +# vim: ft=yaml +# +# Setup variables using grains['osfinger'] based logic. +# You just need to add the key:values for an `osfinger` that differ +# from `defaults.yaml` + `osarch.yaml` + `os_family.yaml` + `osmap.yaml`. +# Only add an `osfinger` which is/will be supported by the formula. +# +# If you do not need to provide defaults via the `os_finger` grain, +# you will need to provide at least an empty dict in this file, e.g. +# osfingermap: {} +--- +# os: Debian +Debian-10: {} ⬇️ Suggested change -Debian-10: {} +Debian-10: + pillar_php_version: "7.3" — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#214 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAFORIWZ5XAWFU77KCXMND3SKXF6VANCNFSM4NYX7DUA> .

-- Regards Wayne Gemmell

myii · 2020-10-14T19:12:58Z

This is actually a great improvement and could resolve a bugreport that I wanted to write.
Is this almost done? What needs to be done to get this merged?

@danny-smit Actually, the first part was waiting for @sticky-note to give some feedback. Otherwise, I would strongly suggest adding the new map.jinja verifier that we've started rolling across formulas (e.g. in the template-formula). That will help ensure that no regressions are introduced when making changes to the map. I might be able to provide something over the coming days.

Thanks for your patience, @waynegemmell.

waynegemmell · 2020-10-14T20:21:39Z

Glad to contribute :)

…

On Wed, Oct 14, 2020 at 9:13 PM Imran Iqbal ***@***.***> wrote: This is actually a great improvement and could resolve a bugreport that I wanted to write. Is this almost done? What needs to be done to get this merged? @danny-smit <https://github.com/danny-smit> Actually, the first part was waiting for @sticky-note <https://github.com/sticky-note> to give some feedback. Otherwise, I would strongly suggest adding the new map.jinja verifier that we've started rolling across formulas (e.g. in the template-formula). That will help ensure that no regressions are introduced when making changes to the map. I *might* be able to provide something over the coming days. Thanks for your patience, @waynegemmell <https://github.com/waynegemmell>. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#214 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAFORIVSLWFZC6FICCVCU5DSKXZ5FANCNFSM4NYX7DUA> .

-- Regards Wayne Gemmell

danny-smit · 2020-10-26T10:35:41Z

@danny-smit Actually, the first part was waiting for @sticky-note to give some feedback. Otherwise, I would strongly suggest adding the new map.jinja verifier that we've started rolling across formulas (e.g. in the template-formula). That will help ensure that no regressions are introduced when making changes to the map. I might be able to provide something over the coming days.

I'm not familiar with that new verifier yet, but is there any way we can help with that?

myii · 2020-10-26T10:38:12Z

I'm not familiar with that new verifier yet, but is there anyway we can help with that?

@danny-smit We're just finalising the verifier to work on Windows as well:

test(map): verify map.jinja dump using _mapdata state openvpn-formula#132

Once that's merged, we'll pretty much have a standard verifier that can be used for most/all formulas. Hopefully sometime this week.

* Prompted by: - saltstack-formulas/php-formula#214 - saltstack-formulas/php-formula#215 - saltstack-formulas/php-formula#216 - saltstack-formulas/php-formula#217

myii · 2021-02-14T13:55:02Z

@waynegemmell @danny-smit It's been a while but then the Travis CI curveball really threw us off. But we're getting on top of things now:

The map.jinja verification files are up and running:
- https://github.com/saltstack-formulas/php-formula/tree/master/test/integration/default/files/_mapdata
We also have the latest platforms being tested in GitLab CI, including ubuntu-20.04.

You'll get all of that if you rebase this PR. Will also need to consider how to deal with #215 at the same time (CC: @sticky-note).

danny-smit · 2021-03-08T16:27:15Z

Is that up to me? Can I do that? Since it is not my pull request.

waynegemmell · 2023-03-22T09:33:24Z

@myii @sticky-note I've done a rebase as recommended but it still seems to be failing. Any idea what's causing this? The logs aren't showing anything that I understand.

make map.jinja use the php_version variable everywhere

waynegemmell requested a review from sticky-note as a code owner June 8, 2020 20:37

crosscodr mentioned this pull request Oct 1, 2020

Add variables for Ubuntu 20.04 #215

Open

19 tasks

danny-smit reviewed Oct 14, 2020

View reviewed changes

This was referenced Oct 26, 2020

[BUG] php.cli tries to write on wrong php.ini file in Ubuntu 20.04 LTS #216

Open

[BUG] On Debian, does not work by default #217

Open

myii mentioned this pull request Dec 22, 2020

test(map): verify map.jinja dump using _mapdata state #221

Merged

19 tasks

myii mentioned this pull request Dec 22, 2020

feat(php): manage map.jinja verification myii/ssf-formula#283

Merged

Wayne Gemmell and others added 4 commits March 22, 2023 11:23

Added osfamilymap, osmap and other defaults

10b7519

Added osfamilymap, osmap and other defaults

859e598

Installs packages in focal.

b22d223

Jammy default version set to 8.1

c24d65e

waynegemmell force-pushed the master branch from 7b6b750 to c24d65e Compare March 22, 2023 09:24

MemoryLeak55 and others added 2 commits June 10, 2024 14:01

make map.jinja use the php_version variable everywhere

3372fab

Merge pull request #1 from MemoryLeak55/master

e6d73ec

make map.jinja use the php_version variable everywhere

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

A start with splitting map.jinja and php works in focal (Ubuntu 20.04) #214

A start with splitting map.jinja and php works in focal (Ubuntu 20.04) #214

waynegemmell commented Jun 8, 2020

pull-assistant bot commented Jun 8, 2020

danny-smit commented Oct 14, 2020

danny-smit Oct 14, 2020

waynegemmell commented Oct 14, 2020 via email

myii commented Oct 14, 2020

waynegemmell commented Oct 14, 2020 via email

danny-smit commented Oct 26, 2020 •

edited

Loading

myii commented Oct 26, 2020 •

edited

Loading

myii commented Feb 14, 2021 •

edited

Loading

danny-smit commented Mar 8, 2021

waynegemmell commented Mar 22, 2023

A start with splitting map.jinja and php works in focal (Ubuntu 20.04) #214

Are you sure you want to change the base?

A start with splitting map.jinja and php works in focal (Ubuntu 20.04) #214

Conversation

waynegemmell commented Jun 8, 2020

PR progress checklist (to be filled in by reviewers)

What type of PR is this?

Primary type

Does this PR introduce a BREAKING CHANGE?

Describe the changes you're proposing

Pillar / config required to test the proposed changes

pull-assistant bot commented Jun 8, 2020

danny-smit commented Oct 14, 2020

danny-smit Oct 14, 2020

Choose a reason for hiding this comment

waynegemmell commented Oct 14, 2020 via email

myii commented Oct 14, 2020

waynegemmell commented Oct 14, 2020 via email

danny-smit commented Oct 26, 2020 • edited Loading

myii commented Oct 26, 2020 • edited Loading

myii commented Feb 14, 2021 • edited Loading

danny-smit commented Mar 8, 2021

waynegemmell commented Mar 22, 2023

Does this PR introduce a `BREAKING CHANGE`?

danny-smit commented Oct 26, 2020 •

edited

Loading

myii commented Oct 26, 2020 •

edited

Loading

myii commented Feb 14, 2021 •

edited

Loading