Skip to content

Commit

Permalink
Merge pull request #5238 from serprex/less-map-bool
Browse files Browse the repository at this point in the history
Replace map[K]bool with map[K]struct{} where it makes sense
  • Loading branch information
openshift-merge-bot[bot] authored Jan 8, 2024
2 parents cb27a94 + a42bfd0 commit a17eb95
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 42 deletions.
8 changes: 5 additions & 3 deletions chroot/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -497,7 +497,7 @@ func TestMounts(t *testing.T) {
})
},
func(t *testing.T, report *types.TestReport) {
foundMounts := make(map[string]bool)
foundBindDestinationMount := false
for _, mount := range report.Spec.Mounts {
if mount.Destination == bind.destination {
allRequired := true
Expand All @@ -516,10 +516,12 @@ func TestMounts(t *testing.T) {
anyRejected = true
}
}
foundMounts[mount.Destination] = allRequired && !anyRejected
if allRequired && !anyRejected {
foundBindDestinationMount = true
}
}
}
if !foundMounts[bind.destination] {
if !foundBindDestinationMount {
t.Errorf("added mount for %s not found with the right flags (%v) in %+v", bind.destination, bind.options, report.Spec.Mounts)
}
},
Expand Down
6 changes: 3 additions & 3 deletions imagebuildah/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -651,7 +651,7 @@ func baseImages(dockerfilenames []string, dockerfilecontents [][]byte, from stri
return nil, fmt.Errorf("reading multiple stages: %w", err)
}
var baseImages []string
nicknames := make(map[string]bool)
nicknames := make(map[string]struct{})
for stageIndex, stage := range stages {
node := stage.Node // first line
for node != nil { // each line
Expand All @@ -673,7 +673,7 @@ func baseImages(dockerfilenames []string, dockerfilecontents [][]byte, from stri
}
}
base := child.Next.Value
if base != "" && base != buildah.BaseImageFakeName && !nicknames[base] {
if base != "" && base != buildah.BaseImageFakeName && !internalUtil.SetHas(nicknames, base) {
headingArgs := argsMapToSlice(stage.Builder.HeadingArgs)
userArgs := argsMapToSlice(stage.Builder.Args)
// append heading args so if --build-arg key=value is not
Expand All @@ -692,7 +692,7 @@ func baseImages(dockerfilenames []string, dockerfilecontents [][]byte, from stri
node = node.Next // next line
}
if stage.Name != strconv.Itoa(stageIndex) {
nicknames[stage.Name] = true
nicknames[stage.Name] = struct{}{}
}
}
return baseImages, nil
Expand Down
47 changes: 24 additions & 23 deletions imagebuildah/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (

"github.com/containers/buildah"
"github.com/containers/buildah/define"
internalUtil "github.com/containers/buildah/internal/util"
"github.com/containers/buildah/pkg/parse"
"github.com/containers/buildah/pkg/sshagent"
"github.com/containers/buildah/util"
Expand Down Expand Up @@ -41,19 +42,19 @@ import (
// complain if we're given values for arguments which have no corresponding ARG
// instruction in the Dockerfile, since that's usually an indication of a user
// error, but for these values we make exceptions and ignore them.
var builtinAllowedBuildArgs = map[string]bool{
"HTTP_PROXY": true,
"http_proxy": true,
"HTTPS_PROXY": true,
"https_proxy": true,
"FTP_PROXY": true,
"ftp_proxy": true,
"NO_PROXY": true,
"no_proxy": true,
"TARGETARCH": true,
"TARGETOS": true,
"TARGETPLATFORM": true,
"TARGETVARIANT": true,
var builtinAllowedBuildArgs = map[string]struct{}{
"HTTP_PROXY": {},
"http_proxy": {},
"HTTPS_PROXY": {},
"https_proxy": {},
"FTP_PROXY": {},
"ftp_proxy": {},
"NO_PROXY": {},
"no_proxy": {},
"TARGETARCH": {},
"TARGETOS": {},
"TARGETPLATFORM": {},
"TARGETVARIANT": {},
}

// Executor is a buildah-based implementation of the imagebuilder.Executor
Expand Down Expand Up @@ -110,8 +111,8 @@ type Executor struct {
forceRmIntermediateCtrs bool
imageMap map[string]string // Used to map images that we create to handle the AS construct.
containerMap map[string]*buildah.Builder // Used to map from image names to only-created-for-the-rootfs containers.
baseMap map[string]bool // Holds the names of every base image, as given.
rootfsMap map[string]bool // Holds the names of every stage whose rootfs is referenced in a COPY or ADD instruction.
baseMap map[string]struct{} // Holds the names of every base image, as given.
rootfsMap map[string]struct{} // Holds the names of every stage whose rootfs is referenced in a COPY or ADD instruction.
blobDirectory string
excludes []string
groupAdd []string
Expand Down Expand Up @@ -278,8 +279,8 @@ func newExecutor(logger *logrus.Logger, logPrefix string, store storage.Store, o
forceRmIntermediateCtrs: options.ForceRmIntermediateCtrs,
imageMap: make(map[string]string),
containerMap: make(map[string]*buildah.Builder),
baseMap: make(map[string]bool),
rootfsMap: make(map[string]bool),
baseMap: make(map[string]struct{}),
rootfsMap: make(map[string]struct{}),
blobDirectory: options.BlobDirectory,
unusedArgs: make(map[string]struct{}),
capabilities: capabilities,
Expand Down Expand Up @@ -606,7 +607,7 @@ func markDependencyStagesForTarget(dependencyMap map[string]*stageDependencyInfo
}

func (b *Executor) warnOnUnsetBuildArgs(stages imagebuilder.Stages, dependencyMap map[string]*stageDependencyInfo, args map[string]string) {
argFound := make(map[string]bool)
argFound := make(map[string]struct{})
for _, stage := range stages {
node := stage.Node // first line
for node != nil { // each line
Expand All @@ -617,12 +618,12 @@ func (b *Executor) warnOnUnsetBuildArgs(stages imagebuilder.Stages, dependencyMa
if strings.Contains(argName, "=") {
res := strings.Split(argName, "=")
if res[1] != "" {
argFound[res[0]] = true
argFound[res[0]] = struct{}{}
}
}
argHasValue := true
if !strings.Contains(argName, "=") {
argHasValue = argFound[argName]
argHasValue = internalUtil.SetHas(argFound, argName)
}
if _, ok := args[argName]; !argHasValue && !ok {
shouldWarn := true
Expand Down Expand Up @@ -772,7 +773,7 @@ func (b *Executor) Build(ctx context.Context, stages imagebuilder.Stages) (image
if err != nil {
return "", nil, fmt.Errorf("while replacing arg variables with values for format %q: %w", base, err)
}
b.baseMap[baseWithArg] = true
b.baseMap[baseWithArg] = struct{}{}
logrus.Debugf("base for stage %d: %q", stageIndex, base)
// Check if selected base is not an additional
// build context and if base is a valid stage
Expand All @@ -794,7 +795,7 @@ func (b *Executor) Build(ctx context.Context, stages imagebuilder.Stages) (image
// was named using argument values, we might
// not record the right value here.
rootfs := strings.TrimPrefix(flag, "--from=")
b.rootfsMap[rootfs] = true
b.rootfsMap[rootfs] = struct{}{}
logrus.Debugf("rootfs needed for COPY in stage %d: %q", stageIndex, rootfs)
// Populate dependency tree and check
// if following ADD or COPY needs any other
Expand Down Expand Up @@ -843,7 +844,7 @@ func (b *Executor) Build(ctx context.Context, stages imagebuilder.Stages) (image
// but also confirm if this is not in additional context.
if _, ok := b.additionalBuildContexts[mountFrom]; !ok {
// Treat from as a rootfs we need to preserve
b.rootfsMap[mountFrom] = true
b.rootfsMap[mountFrom] = struct{}{}
if _, ok := dependencyMap[mountFrom]; ok {
// update current stage's dependency info
currentStageInfo := dependencyMap[stage.Name]
Expand Down
4 changes: 2 additions & 2 deletions imagebuildah/stage_executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -1043,8 +1043,8 @@ func (s *StageExecutor) Execute(ctx context.Context, base string) (imgID string,
moreStages := s.index < len(s.stages)-1
lastStage := !moreStages
onlyBaseImage := false
imageIsUsedLater := moreStages && (s.executor.baseMap[stage.Name] || s.executor.baseMap[strconv.Itoa(stage.Position)])
rootfsIsUsedLater := moreStages && (s.executor.rootfsMap[stage.Name] || s.executor.rootfsMap[strconv.Itoa(stage.Position)])
imageIsUsedLater := moreStages && (internalUtil.SetHas(s.executor.baseMap, stage.Name) || internalUtil.SetHas(s.executor.baseMap, strconv.Itoa(stage.Position)))
rootfsIsUsedLater := moreStages && (internalUtil.SetHas(s.executor.rootfsMap, stage.Name) || internalUtil.SetHas(s.executor.rootfsMap, strconv.Itoa(stage.Position)))

// If the base image's name corresponds to the result of an earlier
// stage, make sure that stage has finished building an image, and
Expand Down
5 changes: 5 additions & 0 deletions internal/util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,3 +97,8 @@ func ExportFromReader(input io.Reader, opts define.BuildOutputOption) error {
}
return nil
}

func SetHas(m map[string]struct{}, k string) bool {
_, ok := m[k]
return ok
}
10 changes: 5 additions & 5 deletions pkg/overlay/overlay.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,10 +113,10 @@ func MountReadOnly(contentDir, source, dest string, rootUID, rootGID int, graphO

// findMountProgram finds if any mount program is specified in the graph options.
func findMountProgram(graphOptions []string) string {
mountMap := map[string]bool{
".mount_program": true,
"overlay.mount_program": true,
"overlay2.mount_program": true,
mountMap := map[string]struct{}{
".mount_program": {},
"overlay.mount_program": {},
"overlay2.mount_program": {},
}

for _, i := range graphOptions {
Expand All @@ -126,7 +126,7 @@ func findMountProgram(graphOptions []string) string {
}
key := s[0]
val := s[1]
if mountMap[key] {
if _, has := mountMap[key]; has {
return val
}
}
Expand Down
12 changes: 6 additions & 6 deletions pkg/parse/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -1053,19 +1053,19 @@ func Device(device string) (string, string, string, error) {
// isValidDeviceMode checks if the mode for device is valid or not.
// isValid mode is a composition of r (read), w (write), and m (mknod).
func isValidDeviceMode(mode string) bool {
var legalDeviceMode = map[rune]bool{
'r': true,
'w': true,
'm': true,
var legalDeviceMode = map[rune]struct{}{
'r': {},
'w': {},
'm': {},
}
if mode == "" {
return false
}
for _, c := range mode {
if !legalDeviceMode[c] {
if _, has := legalDeviceMode[c]; !has {
return false
}
legalDeviceMode[c] = false
delete(legalDeviceMode, c)
}
return true
}
Expand Down

0 comments on commit a17eb95

Please sign in to comment.