-
Notifications
You must be signed in to change notification settings - Fork 42
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
Add EXCLUDE_VERSIONS to explicitly set which versions not to build #20
Conversation
I have also pushed this change to https://github.com/sclorg/container-common-scripts/tree/exclude_ver so that the CI can pickup the commit properly inside sclorg/postgresql-container#182 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally it makes sense to me, just a small comment about grep
..
build.sh
Outdated
@@ -16,6 +16,7 @@ DOCKERFILE_PATH="" | |||
# Perform docker build but append the LABEL with GIT commit id at the end | |||
function docker_build_with_version { | |||
local dockerfile="$1" | |||
[ -n "$(echo "${EXCLUDE_VERSIONS}" | grep ${dir}.${OS})" ] && echo "-> $OS build of version $dir found in EXCLUDED_VERSIONS, skipping build." && return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm just thinking whether we can restrict the regexp a bit, since if we ever had something like such two files Dockerfile.fedora-module
and Dockerfile.fedora
in the repo, while having Dockerfile.fedora-module
in EXCLUDE_VERSIONS
, echo "${EXCLUDE_VERSIONS}" | grep ${dir}.${OS}
would match both Dockerfile.fedora
and Dockerfile.fedora-module
..
grep -e "^\(.*\s\)\?${dir}.${OS}\(\s.*\)\?$"
looks terrible though..
…ding for This is done by creating a .exclude-{rhel7,centos7,fedora} file inside the versioned directory
Modified the change to not look at a |
lgtm |
As there's no other response, I'm merging. But even if there were future complaints -- with this one-liner implementation we can introduce other black-listing methods while keeping this one for compatibility reasons. |
This is connected to #4 . As we are now pushing towards having the whole git history available in a single directory (be-it
latest
or the highest version available so far) to usegit blame
easily, we need to make sure we are able to both keep the Dockerfile.version history when a new version is introduced (which is most likely going to be the latest one) and to make the tests not fail in case centos/rhel packages are still not ready for the new version.Currently to make sure the tests do not fail we would have to remove the Dockerfile.version specific to the distribution which does not yet have the packages available. This would however get rid of the git history of the Dockerfile we have been trying to preserve. This PR aims to fix this problem by introducing
EXCLUDE_VERSIONS
tocommon.mk
andbuild.sh
that can be used to explicitly provide information on which versions for which distribution to ignore.@hhorak @torsava @praiskup WDYT