Skip to content
This repository has been archived by the owner on Nov 9, 2020. It is now read-only.

Fix the issue that installation on Ubuntu Docker host with docker version 17.03.0-ce. #1028

Merged
merged 2 commits into from
Mar 14, 2017

Conversation

lipingxue
Copy link
Contributor

@lipingxue lipingxue commented Mar 13, 2017

Fixes #1023

Tested with fresh Ubuntu 17.03.0-ce install.

@@ -200,7 +200,8 @@ FPM_COMMON := -p $(BIN) \
.PHONY: deb
deb: pkg-prep deb-pkg-prep
@$(CHECK) pkg
$(FPM) --deb-no-default-config-files $(FPM_COMMON) -d '$(DOCKER_PACKAGE) > $(MIN_DOCKER_VERSION)' -t deb .
#$(FPM) --deb-no-default-config-files $(FPM_COMMON) -d '$(DOCKER_PACKAGE) > $(MIN_DOCKER_VERSION)' -t deb .
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, we don't need.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then let's just remove it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While there, let's also remove L213 & L217

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

on the same note: Line#47-49 should be removed as part of this cleanup activity.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the cleanup does not have to happen in this PR. We just need it working first; cleanup wil require more testing and I am not sure it will have any impact on the end result.

Copy link
Contributor

@shaominchen shaominchen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Please remove unused lines.

@@ -200,7 +200,8 @@ FPM_COMMON := -p $(BIN) \
.PHONY: deb
deb: pkg-prep deb-pkg-prep
@$(CHECK) pkg
$(FPM) --deb-no-default-config-files $(FPM_COMMON) -d '$(DOCKER_PACKAGE) > $(MIN_DOCKER_VERSION)' -t deb .
#$(FPM) --deb-no-default-config-files $(FPM_COMMON) -d '$(DOCKER_PACKAGE) > $(MIN_DOCKER_VERSION)' -t deb .
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

on the same note: Line#47-49 should be removed as part of this cleanup activity.

@lipingxue lipingxue force-pushed the fix_docker_install_script.liping branch from ca723fe to 091e90b Compare March 14, 2017 21:10
@lipingxue lipingxue merged commit b9454c2 into master Mar 14, 2017
@msterin msterin deleted the fix_docker_install_script.liping branch March 22, 2017 04:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants