Skip to content
This repository has been archived by the owner on Oct 13, 2023. It is now read-only.

[18.09 backport] fix panic on empty dockerfile #199

Conversation

thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Apr 20, 2019

@thaJeztah thaJeztah added this to the 18.09.6 milestone Apr 20, 2019
@thaJeztah
Copy link
Member Author

ping @tonistiigi @LinuxMercedes ptal

@LinuxMercedes
Copy link

LinuxMercedes commented Apr 20, 2019

The failures looks unrelated:

12:00:36 FAIL: docker_cli_pull_test.go:270: DockerSuite.TestPullWindowsImageFailsOnLinux
12:00:36 
12:00:36 docker_cli_pull_test.go:273:
12:00:36     c.Assert(err.Error(), checker.Contains, "cannot be used on this platform")
12:00:36 ... obtained string = "" +
12:00:36 ...     "\n" +
12:00:36 ...     "Command:  /usr/local/cli/docker pull microsoft/nanoserver\n" +
12:00:36 ...     "ExitCode: 1\n" +
12:00:36 ...     "Error:    exit status 1\n" +
12:00:36 ...     "Stdout:   Using default tag: latest\n" +
12:00:36 ...     "\n" +
12:00:36 ...     "Stderr:   Error response from daemon: manifest for microsoft/nanoserver:latest not found\n" +
12:00:36 ...     "\n" +
12:00:36 ...     "\n" +
12:00:36 ...     "Failures:\n" +
12:00:36 ...     "ExitCode was 1 expected 0\n" +
12:00:36 ...     "Expected no error"
12:00:36 ... substring string = "cannot be used on this platform"

@thaJeztah
Copy link
Member Author

The failures looks unrelated:

Ah, yes, I forgot the fix for that isn't merged yet; the fix for that is included in #198 😓

@LinuxMercedes
Copy link

Ah!

Looks good otherwise! 👍

@thaJeztah thaJeztah force-pushed the 18.09_backport_fix_panic_on_empty_dockerfile branch from b3b89f2 to ea4aef0 Compare April 23, 2019 17:40
@thaJeztah
Copy link
Member Author

rebased, now that #198 was merged

@LinuxMercedes
Copy link

Failures so far look flaky:

@LinuxMercedes
Copy link

Oh, that janky failure is related. You'd need to include moby#38524 or backport docker/docker-py#2216 to whatever version of docker-py is included in this release.

@thaJeztah
Copy link
Member Author

Oh, good catch thanks! opened a separate PR for that one #202

thaJeztah and others added 4 commits June 18, 2019 13:42
Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit c0c05af)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
…branch)

- moby/buildkit#952 [18.09 backport] Have parser error on dockerfiles without instructions
  - backport of moby/buildkit#771 Have parser error on dockerfiles without instructions

Signed-off-by: Sebastiaan van Stijn <[email protected]>
- Wrap parse errors in errdefs.InvalidParameters
- Include dockerfile in error names

Signed-off-by: Natasha Jarus <[email protected]>
(cherry picked from commit 64466b0)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Natasha Jarus <[email protected]>
(cherry picked from commit 18c7e8b)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah thaJeztah force-pushed the 18.09_backport_fix_panic_on_empty_dockerfile branch from ea4aef0 to cd084b2 Compare June 18, 2019 12:44
@thaJeztah
Copy link
Member Author

rebased on top of #202 to get both changes in here

Copy link

@andrewhsu andrewhsu left a comment

Choose a reason for hiding this comment

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

LGTM

@andrewhsu andrewhsu merged commit 70399c4 into docker-archive:18.09 Jun 18, 2019
@thaJeztah thaJeztah deleted the 18.09_backport_fix_panic_on_empty_dockerfile branch June 18, 2019 16:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants