search

openhab / openhab-docker

Repository for building Docker containers for openHAB
https://www.openhab.org/
Eclipse Public License 2.0
212 stars 128 forks source link

Remove auto-generated dockerfiles from repository #250

Closed hakan42 closed 5 years ago

hakan42 commented 5 years ago

During the travis build, all the Dockerfiles are regenerated.

If we would remove the pregenerated ones from the repository, it would be clearer that the real single source of truth is the "update-docker-files.sh" shell script.

So, either remove the dockerfiles or have a BIG BIG COMMENT at the start of the files that they are recreated on every build and should not be relied upon?

wborn commented 5 years ago

I didn't like them being in the repo at first but now I do. It's a good reference to see what's in an image without first having to checkout the repo and then invoke the script. I think it also helps users who want to build slightly modified Docker images. That way they don't need to figure out how our tooling works.

the real single source of truth is the "update-docker-files.sh" shell script.

The single source of truth is "update.sh". :wink:

The docker files already have a BIG BIG COMMENT. Maybe it should also be added to the other files. Some contributors still overlook this big comment.

hakan42 commented 5 years ago

Ah, sorry, did not notice the BIG BIG COMMENT 😁

I always tend to get a little bit concerned when machine-generated stuff ends up committed to the source tree, but I can also see the reasoning with "without having to invoke the script...".

Maybe run a "git diff" after cloning on travis (and breaking the build if diffs are found?) so we can make really really sure that what we are building is precicely what is in the repo? If this is agreeable, I would come up with an appropriate PR.

wborn commented 5 years ago

If this is agreeable, I would come up with an appropriate PR.

It would be a good idea to fail the build with a helpful message when there are changes after invoking the update.sh script. :+1:

It would also help if we explain the generation using update.sh somewhere in the README.md. :slightly_smiling_face:

hakan42 commented 5 years ago

o.k., in that case, I have something to do during the weekend or at some late night hacking session :)

wborn commented 5 years ago

Another benefit of having the generated files in the repo is that it's easier to review script changes. If the generated content isn't there, it will be harder to see how script changes impact the generated content.

cniweb commented 5 years ago

I agree with wborn. Can we close this Issue?

hakan42 commented 5 years ago

Sure, I just kept it open as a reminder for me that I wanted to come up with a PR for checking during build as mentioned by wborn...