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
restore support for --memory
#10492
restore support for --memory
#10492
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## v2 #10492 +/- ##
===========================================
- Coverage 59.90% 19.55% -40.35%
===========================================
Files 105 104 -1
Lines 9056 9102 +46
===========================================
- Hits 5425 1780 -3645
- Misses 3066 7135 +4069
+ Partials 565 187 -378
... and 72 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
98b6a28
to
e5c9941
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just went pedantic on the text as usual 😅
cmd/compose/build.go
Outdated
@@ -125,8 +123,7 @@ func buildCommand(p *ProjectOptions, streams api.Streams, backend api.Service) * | |||
cmd.Flags().BoolVar(&opts.noCache, "no-cache", false, "Do not use cache when building the image") | |||
cmd.Flags().Bool("no-rm", false, "Do not remove intermediate containers after a successful build. DEPRECATED") | |||
cmd.Flags().MarkHidden("no-rm") //nolint:errcheck | |||
cmd.Flags().StringVarP(&opts.memory, "memory", "m", "", "Set memory limit for the build container. Not supported on buildkit yet.") | |||
cmd.Flags().MarkHidden("memory") //nolint:errcheck | |||
cmd.Flags().VarP(&opts.memory, "memory", "m", "Set memory limit for the build container. Not supported on buildkit yet.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cmd.Flags().VarP(&opts.memory, "memory", "m", "Set memory limit for the build container. Not supported on buildkit yet.") | |
cmd.Flags().VarP(&opts.memory, "memory", "m", "Set memory limit for the build container. Not supported by BuildKit.") |
pkg/compose/build.go
Outdated
@@ -102,6 +102,11 @@ func (s *composeService) build(ctx context.Context, project *types.Project, opti | |||
} | |||
return nil | |||
} | |||
|
|||
if options.Memory != 0 { | |||
fmt.Fprintln(s.stderr(), "WARNING --memory is ignored as not supported in buildkit.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fmt.Fprintln(s.stderr(), "WARNING --memory is ignored as not supported in buildkit.") | |
fmt.Fprintln(s.stderr(), "WARNING: --memory is not supported by BuildKit and will be ignored.") |
2b3b704
to
e33986a
Compare
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
What I did
restore support for
build --memory
on classic builder.This flag was hidden as initial design only supported buildkit, but as we introduced support for classic builder, there's no reason not so enable use of this flag
Related issue
closes #10488
(not mandatory) A picture of a cute animal, if possible in relation to what you did