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

fix: error building with Dockerfile.contrib #261

Closed
wants to merge 1 commit into from

Conversation

ahochsteger
Copy link
Collaborator

Fixes the following error when building a docker image using Dockerfile.contrib:

Step 10/39 : COPY --chown=node node-zwave-js /home/node/node-zwave-js
COPY failed: stat /var/lib/docker/tmp/docker-builder759222654/node-zwave-js: no such file or directory

@coveralls
Copy link

Pull Request Test Coverage Report for Build 493791282

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 23.579%

Totals Coverage Status
Change from base Build 493742410: 0.0%
Covered Lines: 1958
Relevant Lines: 8483

💛 - Coveralls

@robertsLando
Copy link
Member

@larstobi Give this a check when you have time :)

Copy link
Member

@robertsLando robertsLando left a comment

Choose a reason for hiding this comment

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

LGTM

@larstobi
Copy link
Contributor

Trying

@larstobi
Copy link
Contributor

Please don't merge this yet. I have some comments.

@@ -11,8 +11,8 @@ RUN git clone -b ${Z2M_BRANCH} --depth 1 https://github.com/zwave-js/zwavejs2mqt

##### LOCAL SOURCE #####
FROM node:erbium-buster AS local-copy-src
COPY --chown=node node-zwave-js /home/node/node-zwave-js
COPY --chown=node zwavejs2mqtt /home/node/zwavejs2mqtt
COPY --from=git-clone-src --chown=node /home/node/node-zwave-js /home/node/node-zwave-js
Copy link
Contributor

Choose a reason for hiding this comment

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

This will not work when using local-copy-src since it will take the cloned source

@@ -26,23 +26,23 @@ RUN yarn install --network-timeout=${YARN_NETWORK_TIMEOUT}
RUN yarn run build:full
RUN yarn install --production --frozen-lockfile
RUN for i in config core serial shared; do \
cd packages/$i && \
yarn version --no-git-tag-version --new-version $(yarn versions --json| \
jq -r '[.data."@zwave-js/'${i}'"]'[0])-$(git rev-parse --short HEAD) && \
Copy link
Contributor

Choose a reason for hiding this comment

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

The reason this is indented is that it's part of the previous command. I think that's a good thing.

Copy link
Collaborator Author

@ahochsteger ahochsteger Jan 19, 2021

Choose a reason for hiding this comment

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

The reason this is indented is that it's part of the previous command. I think that's a good thing.

I think, this was by mistake due to auto-format on save which I seem to have missed.

@@ -26,23 +26,23 @@ RUN yarn install --network-timeout=${YARN_NETWORK_TIMEOUT}
RUN yarn run build:full
RUN yarn install --production --frozen-lockfile
RUN for i in config core serial shared; do \
cd packages/$i && \
Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer not reducing indentation spaces from 4 to 2. It is also consistent with the other Dockerfile in this directory.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I would prefer not reducing indentation spaces from 4 to 2. It is also consistent with the other Dockerfile in this directory.

You're right - this was slipped through as well due to auto-format on save.

@larstobi
Copy link
Contributor

@ahochsteger Thanks for taking the time to raise this PR! :-)

I've investigated your changes, and unfortunately I cannot say that I agree.

The reason you're getting that error is that you're not prefixing your docker command with DOCKER_BUILDKIT=1

The changes you've made breaks the Dockerfile when using SRC=local-copy-src so I think this should not be merged.

Now, I understand that you would like to use this dockerfile without enabling docker BuildKit, and I've investigated with some ways by making another build stage, called "start", but I still haven't been able to make docker skip unused stages without BuildKit.

If you can find a way that works for both SRC arguments (local and clone), without using BuildKit, then I'm all for it!

@ahochsteger
Copy link
Collaborator Author

@larstobi thangs for taking the time to review this PR.
It seems, that I've not read the README which documents how to use it properly ;-).
I'm fine to cancel this PR and remove the branch, since it's working for me when using it the right way.

@larstobi
Copy link
Contributor

Great to hear that it works for you now! :-D

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