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

Make sure we generate a proper docker tag #1675

Closed

Conversation

krnowak
Copy link
Contributor

@krnowak krnowak commented Jul 14, 2016

I have a branch named like /, which becomes a part of
the tag name. Docker complains about it, because / is not a valid
character in the tag.

This commit replaces all sequences of invalid chars with a single dash
and makes sure that the tag name never exceeds the 127 chars limit.

@alban
Copy link
Contributor

alban commented Jul 14, 2016

This is similar to #1655

And probably failing for the same reason.

@tomwilkie
Copy link
Contributor

Hi @krnowak; can you rebase this on top of master? It should pass now.

@krnowak krnowak force-pushed the krnowak/fix-image-tag branch from d2c57bc to b162921 Compare July 21, 2016 11:21
@krnowak krnowak force-pushed the krnowak/fix-image-tag branch from b162921 to e8e5837 Compare August 4, 2016 09:49
@krnowak
Copy link
Contributor Author

krnowak commented Aug 4, 2016

Updated, lint seems to fail with some typos. For lint fixes please see #1751.

@@ -8,5 +8,10 @@ WORKING_SUFFIX=$(if ! git diff --exit-code --quiet HEAD >&2; \
then echo "-WIP"; \
else echo ""; \
fi)
BRANCH_PREFIX=$(git rev-parse --abbrev-ref HEAD)
echo "${BRANCH_PREFIX//\//-}-$(git rev-parse --short HEAD)$WORKING_SUFFIX"

This comment was marked as abuse.

@krnowak krnowak force-pushed the krnowak/fix-image-tag branch from e8e5837 to 559c62e Compare August 9, 2016 08:52
This commit replaces all sequences of invalid chars with a single dash
and makes sure that the tag name never exceeds the 127 chars limit.
@krnowak krnowak force-pushed the krnowak/fix-image-tag branch from 559c62e to a677da2 Compare August 9, 2016 09:37
@krnowak
Copy link
Contributor Author

krnowak commented Aug 9, 2016

Updated, so we can be sure that tag names in circleCI are also valid.

@2opremio
Copy link
Contributor

@krnowak / @alban Do we still need this?

@krnowak
Copy link
Contributor Author

krnowak commented Nov 16, 2016

Up to you, really. But now I guess this PR would need to be split in two - one for image-tag to be filed in build-tools repository and one for scope importing the new build-tools version and modifying circle.yml.

My issue was the same as the one @alban had - I had / in my branch name and docker did not like it. I just made extra sure that the tag we generate based on a branch name is always valid (no invalid chars and shorter than 128 chars).

@tomwilkie
Copy link
Contributor

Already an image-tag in build-tools: https://github.com/weaveworks/build-tools/blob/master/image-tag

@2opremio 2opremio closed this Dec 5, 2016
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.

4 participants