s2i-python-container icon indicating copy to clipboard operation
s2i-python-container copied to clipboard

New generator for distgen working with minimal images

Open frenzymadness opened this issue 1 year ago • 3 comments

Python replacement for the black magic in https://github.com/sclorg/container-common-scripts/blob/233496946a028218958d8fbb1857f921ad8a99ef/common.mk#L147-L164 and in https://github.com/sclorg/container-common-scripts/blob/master/generate.sh

It might be possible to merge some of the files specific for the minimal container to the original one but that can be done in the future. This does not apply to README.md which is completely different.

frenzymadness avatar Aug 11 '22 10:08 frenzymadness

[test]

frenzymadness avatar Aug 11 '22 10:08 frenzymadness

I am wondering, can we add the black magic replacements to container-common-scripts, or are they too specific for the python repository to be used elsewhere?

pkubatrh avatar Aug 11 '22 10:08 pkubatrh

I am wondering, can we add the black magic replacements to container-common-scripts, or are they too specific for the python repository to be used elsewhere?

I'd like to replace what we have in the common scripts with this after some testing here. How many images use distgen? I know only about postgres.

frenzymadness avatar Aug 11 '22 11:08 frenzymadness

Meta thought from meeting with Lumir: Currently in multispec it's necessary to exclude version/distro combinations that are unwanted. How about switching it around and only declaring version/distro combinations that are wanted? It feels more natural and increases readability.

torsava avatar Aug 16 '22 11:08 torsava

Meta thought from meeting with Lumir: Currently in multispec it's necessary to exclude version/distro combinations that are unwanted. How about switching it around and only declaring version/distro combinations that are wanted? It feels more natural and increases readability.

This is not possible in distgen, see: https://github.com/devexp-db/distgen/blob/main/docs/spec_multispec.rst#multispecs

But, we might be able to implement it outside of distgen. The current workflow takes all the possible combinations from multispec.yaml (generating all of them, excluding specified). But we use them one by one only for files for which we need them for so we might also come with some external yaml config containing all the wanted combinations and remove the matrix section from distgen. Or, we might try to implement this in distgen itself.

frenzymadness avatar Aug 16 '22 13:08 frenzymadness

First review pass finished. Overall it's looking much better than the previous machinery, I like it!

torsava avatar Aug 16 '22 15:08 torsava

Meta thought from meeting with Lumir: Currently in multispec it's necessary to exclude version/distro combinations that are unwanted. How about switching it around and only declaring version/distro combinations that are wanted? It feels more natural and increases readability.

This is not possible in distgen, see: https://github.com/devexp-db/distgen/blob/main/docs/spec_multispec.rst#multispecs

But, we might be able to implement it outside of distgen. The current workflow takes all the possible combinations from multispec.yaml (generating all of them, excluding specified). But we use them one by one only for files for which we need them for so we might also come with some external yaml config containing all the wanted combinations and remove the matrix section from distgen. Or, we might try to implement this in distgen itself.

Good to know. That'd be a great idea for the future, but no need to burden this PR with it.

torsava avatar Aug 17 '22 11:08 torsava

Meta thought from meeting with Lumir: Currently in multispec it's necessary to exclude version/distro combinations that are unwanted. How about switching it around and only declaring version/distro combinations that are wanted? It feels more natural and increases readability.

This is not possible in distgen, see: https://github.com/devexp-db/distgen/blob/main/docs/spec_multispec.rst#multispecs

But, we might be able to implement it outside of distgen. The current workflow takes all the possible combinations from multispec.yaml (generating all of them, excluding specified). But we use them one by one only for files for which we need them for so we might also come with some external yaml config containing all the wanted combinations and remove the matrix section from distgen. Or, we might try to implement this in distgen itself.

Good to know. That'd be a great idea for the future, but no need to burden this PR with it.

I've filled an issue in distgen and it seems that they're open to this idea. Will see how hard it'll be.

frenzymadness avatar Aug 19 '22 22:08 frenzymadness

See the new commits - some of them are fixup but it's better for review. [test]

frenzymadness avatar Aug 23 '22 13:08 frenzymadness

Rebased. I'm happy with how the commit history looks now.

frenzymadness avatar Aug 31 '22 12:08 frenzymadness

[test]

frenzymadness avatar Aug 31 '22 12:08 frenzymadness

This pull request affects also https://github.com/sclorg/container-common-scripts/pull/275 and varnish-container and postgresql-container which uses manifest.sh

phracek avatar Sep 05 '22 08:09 phracek

I've done a sanity check of all the commits again. All the previous discussions were successfully addressed. I've pinpointed one typo, but besides that all looks good to me!

However, be warned that if you remove CCCP, Putin might attack you in the next ~30 years. Beware! :smiley:

torsava avatar Sep 06 '22 16:09 torsava