From e8e83bb8bd778fdf260cef16ca1a9ec3edf307e9 Mon Sep 17 00:00:00 2001 From: Nalin Dahyabhai Date: Tue, 30 Jul 2024 15:42:50 -0400 Subject: [PATCH] Update some godocs, use 0o to prefix an octal in a comment Update some godocs, and update an octal value in a godoc to start with 0o instead of just 0, to match the literal on the next line. Signed-off-by: Nalin Dahyabhai --- define/build.go | 9 ++++----- imagebuildah/stage_executor.go | 17 ++++++++++------- internal/types.go | 12 ++++++------ internal/volumes/volumes.go | 17 +++++++++-------- 4 files changed, 29 insertions(+), 26 deletions(-) diff --git a/define/build.go b/define/build.go index b50f26b61f3..1ea871c51ab 100644 --- a/define/build.go +++ b/define/build.go @@ -19,12 +19,11 @@ type AdditionalBuildContext struct { IsURL bool // Value is the name of an image which may or may not have already been pulled. IsImage bool - // Value holds a URL, an image name, or an absolute filesystem path. + // Value holds a URL (if IsURL), an image name (if IsImage), or an absolute filesystem path. Value string - // Absolute filesystem path to downloaded and exported build context - // from external tar archive. This will be populated only if following - // buildcontext is created from IsURL and was downloaded before in any - // of the RUN step. + // Absolute filesystem path to a downloaded and exported build context + // from an external tar archive. This will be populated only if the + // build context was a URL and its contents have been downloaded. DownloadedCache string } diff --git a/imagebuildah/stage_executor.go b/imagebuildah/stage_executor.go index 617117a57ac..e55c4e142a9 100644 --- a/imagebuildah/stage_executor.go +++ b/imagebuildah/stage_executor.go @@ -623,10 +623,8 @@ func (s *StageExecutor) performCopy(excludes []string, copies ...imagebuilder.Co return nil } -// Returns a map of StageName/ImageName:internal.StageMountDetails for RunOpts if any --mount with from is provided -// Stage can automatically cleanup this mounts when a stage is removed -// check if RUN contains `--mount` with `from`. If yes pre-mount images or stages from executor for Run. -// stages mounted here will we used be Run(). +// Returns a map of StageName/ImageName:internal.StageMountDetails for the +// items in the passed-in mounts list which include a "from=" value. func (s *StageExecutor) runStageMountPoints(mountList []string) (map[string]internal.StageMountDetails, error) { stageMountPoints := make(map[string]internal.StageMountDetails) for _, flag := range mountList { @@ -649,9 +647,11 @@ func (s *StageExecutor) runStageMountPoints(mountList []string) (map[string]inte if fromErr != nil { return nil, fmt.Errorf("unable to resolve argument %q: %w", val, fromErr) } - // If additional buildContext contains this - // give priority to that and break if additional - // is not an external image. + // If the value corresponds to an additional build context, + // the mount source is either either the rootfs of the image, + // the filesystem path, or a temporary directory populated + // with the contents of the URL, all in preference to any + // stage which might have the value as its name. if additionalBuildContext, ok := s.executor.additionalBuildContexts[from]; ok { if additionalBuildContext.IsImage { mountPoint, err := s.getImageRootfs(s.ctx, additionalBuildContext.Value) @@ -703,10 +703,13 @@ func (s *StageExecutor) runStageMountPoints(mountList []string) (map[string]inte if isStage, err := s.executor.waitForStage(s.ctx, from, s.stages[:s.index]); isStage && err != nil { return nil, err } + // If the source's name is a stage, return a + // pointer to its rootfs. if otherStage, ok := s.executor.stages[from]; ok && otherStage.index < s.index { stageMountPoints[from] = internal.StageMountDetails{IsStage: true, DidExecute: otherStage.didExecute, MountPoint: otherStage.mountPoint} break } else { + // Treat the source's name as the name of an image. mountPoint, err := s.getImageRootfs(s.ctx, from) if err != nil { return nil, fmt.Errorf("%s from=%s: no stage or image found with that name", flag, from) diff --git a/internal/types.go b/internal/types.go index ee87eca2255..1c8ef724344 100644 --- a/internal/types.go +++ b/internal/types.go @@ -1,18 +1,18 @@ package internal const ( - // Temp directory which stores external artifacts which are download for a build. - // Example: tar files from external sources. + // BuildahExternalArtifactsDir is the pattern passed to os.MkdirTemp() + // to generate a temporary directory which will be used to hold + // external items which are downloaded for a build, typically a tarball + // being used as an additional build context. BuildahExternalArtifactsDir = "buildah-external-artifacts" ) -// Types is internal packages are suspected to change with releases avoid using these outside of buildah - // StageMountDetails holds the Stage/Image mountpoint returned by StageExecutor // StageExecutor has ability to mount stages/images in current context and // automatically clean them up. type StageMountDetails struct { DidExecute bool // tells if the stage which is being mounted was freshly executed or was part of older cache - IsStage bool // tells if mountpoint returned from stage executor is stage or image - MountPoint string // mountpoint of stage/image + IsStage bool // true if the mountpoint is a temporary directory or a stage's rootfs, false if it's an image + MountPoint string // mountpoint of the stage or image's root directory } diff --git a/internal/volumes/volumes.go b/internal/volumes/volumes.go index ac77fb4f523..3d20ccb6899 100644 --- a/internal/volumes/volumes.go +++ b/internal/volumes/volumes.go @@ -239,7 +239,7 @@ func GetCacheMount(args []string, _ storage.Store, _ string, additionalMountPoin } // if id is set a new subdirectory with `id` will be created under /host-temp/buildah-build-cache/id id := "" - // buildkit parity: cache directory defaults to 755 + // buildkit parity: cache directory defaults to 0o755 mode = 0o755 // buildkit parity: cache directory defaults to uid 0 if not specified uid := 0 @@ -345,8 +345,9 @@ func GetCacheMount(args []string, _ storage.Store, _ string, additionalMountPoin } if fromStage != "" { - // do not create cache on host - // instead use read-only mounted stage as cache + // do not create and use a cache directory on the host, + // instead use the location in the mounted stage or + // temporary directory as the cache mountPoint := "" if additionalMountPoints != nil { if val, ok := additionalMountPoints[fromStage]; ok { @@ -355,17 +356,17 @@ func GetCacheMount(args []string, _ storage.Store, _ string, additionalMountPoin } } } - // Cache does not supports using image so if not stage found - // return with error + // Cache does not support using an image so if there's no such + // stage or temporary directory, return an error if mountPoint == "" { return newMount, nil, fmt.Errorf("no stage found with name %s", fromStage) } // path should be /contextDir/specified path newMount.Source = filepath.Join(mountPoint, filepath.Clean(string(filepath.Separator)+newMount.Source)) } else { - // we need to create cache on host if no image is being used + // we need to create the cache directory on the host if no image is being used - // since type is cache and cache can be reused by consecutive builds + // since type is cache and a cache can be reused by consecutive builds // create a common cache directory, which persists on hosts within temp lifecycle // add subdirectory if specified @@ -388,7 +389,7 @@ func GetCacheMount(args []string, _ storage.Store, _ string, additionalMountPoin UID: uid, GID: gid, } - // buildkit parity: change uid and gid if specified otheriwise keep `0` + // buildkit parity: change uid and gid if specified, otherwise keep `0` err = idtools.MkdirAllAndChownNew(newMount.Source, os.FileMode(mode), idPair) if err != nil { return newMount, nil, fmt.Errorf("unable to change uid,gid of cache directory: %w", err)