Check if files generated by dist-gen are generated properly

sclorg / container-common-scripts

Apache License 2.0

20 stars 45 forks source link

Check if files generated by dist-gen are generated properly #275

Closed phracek closed 1 year ago

phracek commented 2 years ago

This new function checks if files generated by dist-gen are committed into each pull request properly. The function is called only if the manifest.sh file is present in the root directory of a container.

The command which checks the status of generated files is:

git diff --ignore-submodules=all --exit-code

the common submodule is not checked at all.

Nowadays we have three containers that use dist-gen generated sources:

varnish-container
postgresql-container
s2i-python-container

The old PR has been closed because it had no reference https://github.com/sclorg/container-common-scripts/pull/218

Signed-off-by: Petr "Stone" Hracek phracek@redhat.com

phracek commented 2 years ago

[test]

phracek commented 2 years ago

@pkubatrh Would you be willing to review it?

zmiklank commented 2 years ago

Would this work in python container, where files for minimal container are generated manually, and not by the manifest.sh rules?

I tried that and proceeded with the steps hinted in this PR:

'make clean-versions'
'make generate-all'

and I deleted the whole 3.9-minimal directory.

frenzymadness commented 2 years ago

I see two ways forward here:

Implement the new generator everywhere - That'd mean taking the PR from Python container repo, implementing it in the common scripts and switching all the containers using distgen at once. This is kinda risky but allows this PR to work with all of the containers at once.
Ignore Python for now - Finish this PR and ignore Python container for now (don't detect manifest.yml). We'll test the new generator for some time only for Python container and when we think it's ready, we can then move it to the common scripts and change the detection mechanism here as well.

What do you think and prefer?

zmiklank commented 2 years ago

I see two ways forward here:

* Implement the new generator everywhere - That'd mean taking the PR from Python container repo, implementing it in the common scripts and switching all the containers using distgen at once. This is kinda risky but allows this PR to work with all of the containers at once.

* Ignore Python for now - Finish this PR and ignore Python container for now (don't detect `manifest.yml`). We'll test the new generator for some time only for Python container and when we think it's ready, we can then move it to the common scripts and change the detection mechanism here as well.

What do you think and prefer?

I agree and prefer 2).

phracek commented 2 years ago

Rebased

[test]

phracek commented 1 year ago

Closing we will have different pull request