Skip to content
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

Merged
merged 1 commit into from
Aug 17, 2017

Conversation

pkubatrh
Copy link
Member

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 use git 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 to common.mk and build.sh that can be used to explicitly provide information on which versions for which distribution to ignore.

@hhorak @torsava @praiskup WDYT

@pkubatrh
Copy link
Member Author

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

Copy link
Member

@hhorak hhorak left a 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
Copy link
Member

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
@pkubatrh
Copy link
Member Author

Modified the change to not look at a make argument but instead check if a .exclude-$OS file is present in the versioned directory. If it exists, do not build the corresponding distribution version.

@praiskup
Copy link
Contributor

lgtm

@praiskup
Copy link
Contributor

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.

@praiskup praiskup merged commit 66bcdc2 into sclorg:master Aug 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants