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

dockerfile: add syntax directive for introducing new features #384

Merged
merged 1 commit into from
May 17, 2018

Conversation

tonistiigi
Copy link
Member

Add syntax directive for specifying the builder reference so that new dockerfile features can be targeted. New Dockerfile features can be added now without breaking backward compatibility or requiring users to upgrade. The base one would remain compatible with current moby/docker.

@tiborvass

Signed-off-by: Tonis Tiigi tonistiigi@gmail.com

@tonistiigi tonistiigi changed the base branch from master to llbsolver-next May 14, 2018 22:40
m := map[string]string{}
s := bufio.NewScanner(r)
for s.Scan() {
match := reDirective.FindStringSubmatch(s.Text())
Copy link
Collaborator

Choose a reason for hiding this comment

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

Needs to handle uppercase too, so it should be strings.ToLower(s.Text()) or reDirective.FindSubmatch(bytes.ToLower(s.Bytes())) (and then cast string(m) and string(match[2]))

Copy link
Collaborator

Choose a reason for hiding this comment

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

Or just change the regexp.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated

@@ -116,6 +116,13 @@ func Build(ctx context.Context, c client.Client) error {
return err
}

if _, ok := c.Opts()["cmdline"]; !ok {
ref, argv, ok := dockerfile2llb.DetectSyntax(bytes.NewBuffer(dtDockerfile))
Copy link
Member

Choose a reason for hiding this comment

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

nit: I expect argv to be a vector (slice)

Copy link
Member Author

Choose a reason for hiding this comment

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

Do I just rename the var to cmdline?

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
@tiborvass
Copy link
Collaborator

LGTM

@AkihiroSuda
Copy link
Member

#382 needs to be merged first?

@AkihiroSuda AkihiroSuda changed the base branch from llbsolver-next to master May 17, 2018 02:16
@AkihiroSuda
Copy link
Member

AkihiroSuda commented May 17, 2018

merging into master (#382 llbsolver-next is merged now)

@AkihiroSuda AkihiroSuda merged commit 10003f2 into moby:master May 17, 2018
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