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

feat: Adding a preserve mtime feature to copy_to_directory and copy_directory #898

Merged
merged 1 commit into from
Aug 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion docs/copy_directory.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 4 additions & 2 deletions docs/copy_to_directory.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

25 changes: 25 additions & 0 deletions e2e/smoke/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -99,3 +99,28 @@ bats_test(
"basic.bats",
],
)

copy_to_directory(
name = "copy_to_directory_mtime_case",
srcs = ["d"],
out = "copy_to_directory_mtime_out",
preserve_mtime = True,
)

copy_directory(
name = "copy_directory_mtime_case",
src = "d",
out = "copy_directory_mtime_out",
preserve_mtime = True,
)

sh_test(
name = "test_preserve_mtime",
srcs = ["test_preserve_mtime.sh"],
data = [
"d",
":copy_to_directory_mtime_case",
":copy_directory_mtime_case",
],
size = "small",
)
30 changes: 30 additions & 0 deletions e2e/smoke/test_preserve_mtime.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
#!/usr/bin/env bash

set -euo pipefail

function main {
compareMTimes d/1 copy_to_directory_mtime_out/d/1
compareMTimes d/1 copy_directory_mtime_out/1
}

function compareMTimes {
local originalFile="$1"
local copiedFile="$2"

local mtimeOriginal
mtimeOriginal="$(stat --dereference --format=%y "$originalFile")"

local mtimeCopy
mtimeCopy="$(stat --dereference --format=%y "$copiedFile")"

if [[ "$mtimeOriginal" != "$mtimeCopy" ]]; then
echo "Preserve mtime test failed. Modify times do not match for $originalFile and $copiedFile"
echo " Original modify time: $mtimeOriginal"
echo " Copied modify time: $mtimeCopy"
return 1
fi

echo "Preserve mtime test passed for $originalFile and $copiedFile"
}

main "$@"
13 changes: 12 additions & 1 deletion lib/private/copy_directory.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ def copy_directory_bin_action(
dst,
copy_directory_bin,
hardlink = "auto",
verbose = False):
verbose = False,
preserve_mtime = False):
"""Factory function that creates an action to copy a directory from src to dst using a tool binary.

The tool binary will typically be the `@aspect_bazel_lib//tools/copy_directory` `go_binary`
Expand All @@ -35,6 +36,8 @@ def copy_directory_bin_action(
See copy_directory rule documentation for more details.

verbose: If true, prints out verbose logs to stdout

preserve_mtime: If true, preserve the modified time from the source.
"""
args = [
src.path,
Expand All @@ -48,6 +51,9 @@ def copy_directory_bin_action(
elif hardlink == "auto" and not src.is_source:
args.append("--hardlink")

if preserve_mtime:
args.append("--preserve-mtime")

ctx.actions.run(
inputs = [src],
outputs = [dst],
Expand All @@ -71,6 +77,7 @@ def _copy_directory_impl(ctx):
copy_directory_bin = copy_directory_bin,
hardlink = ctx.attr.hardlink,
verbose = ctx.attr.verbose,
preserve_mtime = ctx.attr.preserve_mtime,
)

return [
Expand All @@ -93,6 +100,10 @@ _copy_directory = rule(
default = "auto",
),
"verbose": attr.bool(),
"preserve_mtime": attr.bool(
doc = "If True, the last modified time of copied files is preserved.",
default = False,
),
# use '_tool' attribute for development only; do not commit with this attribute active since it
# propagates a dependency on rules_go which would be breaking for users
# "_tool": attr.label(
Expand Down
10 changes: 10 additions & 0 deletions lib/private/copy_to_directory.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ removed from sources files.
- `off`: all files are copied
- `on`: hardlinks are used for all files (not recommended)
""",
"preserve_mtime": """If True, the last modified time of copied files is preserved.""",
# verbose
"verbose": """If true, prints out verbose logs to stdout""",
}
Expand Down Expand Up @@ -251,6 +252,10 @@ _copy_to_directory_attr = {
default = "auto",
doc = _copy_to_directory_attr_doc["hardlink"],
),
"preserve_mtime": attr.bool(
default = False,
doc = _copy_to_directory_attr_doc["preserve_mtime"],
),
"verbose": attr.bool(
doc = _copy_to_directory_attr_doc["verbose"],
),
Expand Down Expand Up @@ -285,6 +290,7 @@ def _copy_to_directory_impl(ctx):
replace_prefixes = ctx.attr.replace_prefixes,
allow_overwrites = ctx.attr.allow_overwrites,
hardlink = ctx.attr.hardlink,
preserve_mtime = ctx.attr.preserve_mtime,
verbose = ctx.attr.verbose,
)

Expand Down Expand Up @@ -324,6 +330,7 @@ def copy_to_directory_bin_action(
replace_prefixes = {},
allow_overwrites = False,
hardlink = "auto",
preserve_mtime = False,
verbose = False):
"""Factory function to copy files to a directory using a tool binary.

Expand Down Expand Up @@ -382,6 +389,8 @@ def copy_to_directory_bin_action(

See copy_to_directory rule documentation for more details.

preserve_mtime: If true, preserve the modified time from the source.

verbose: If true, prints out verbose logs to stdout
"""

Expand Down Expand Up @@ -472,6 +481,7 @@ def copy_to_directory_bin_action(
"include_srcs_patterns": include_srcs_patterns,
"replace_prefixes": replace_prefixes,
"root_paths": root_paths,
"preserve_mtime": preserve_mtime,
"verbose": verbose,
}

Expand Down
24 changes: 17 additions & 7 deletions tools/common/copy.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"log"
"os"
"sync"
"time"
)

// From https://opensource.com/article/18/6/copying-files-go
Expand All @@ -27,7 +28,7 @@ func CopyFile(src string, dst string) error {
}

func Copy(opts CopyOpts) {
if !opts.info.Mode().IsRegular() {
if !opts.srcInfo.Mode().IsRegular() {
log.Fatalf("%s is not a regular file", opts.src)
}

Expand Down Expand Up @@ -79,6 +80,14 @@ func Copy(opts CopyOpts) {
if err != nil {
log.Fatal(err)
}

if opts.preserveMTime {
accessTime := time.Now()
err := os.Chtimes(opts.dst, accessTime, opts.srcInfo.ModTime())
if err != nil {
log.Fatal(err)
}
}
}

type CopyWorker struct {
Expand All @@ -97,12 +106,13 @@ func (w *CopyWorker) Run(wg *sync.WaitGroup) {
}

type CopyOpts struct {
src, dst string
info fs.FileInfo
hardlink bool
verbose bool
src, dst string
srcInfo fs.FileInfo
hardlink bool
verbose bool
preserveMTime bool
}

func NewCopyOpts(src string, dst string, info fs.FileInfo, hardlink bool, verbose bool) CopyOpts {
return CopyOpts{src: src, dst: dst, info: info, hardlink: hardlink, verbose: verbose}
func NewCopyOpts(src string, dst string, srcInfo fs.FileInfo, hardlink bool, verbose bool, preserveMTime bool) CopyOpts {
return CopyOpts{src: src, dst: dst, srcInfo: srcInfo, hardlink: hardlink, verbose: verbose, preserveMTime: preserveMTime}
}
9 changes: 6 additions & 3 deletions tools/copy_directory/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ type pathSet map[string]bool
var srcPaths = pathSet{}
var hardlink = false
var verbose = false
var preserveMTime = false

type walker struct {
queue chan<- common.CopyOpts
Expand Down Expand Up @@ -68,13 +69,13 @@ func (w *walker) copyDir(src string, dst string) error {
return w.copyDir(linkPath, d)
} else {
// symlink points to a regular file
w.queue <- common.NewCopyOpts(linkPath, d, stat, hardlink, verbose)
w.queue <- common.NewCopyOpts(linkPath, d, stat, hardlink, verbose, preserveMTime)
return nil
}
}

// a regular file
w.queue <- common.NewCopyOpts(p, d, info, hardlink, verbose)
w.queue <- common.NewCopyOpts(p, d, info, hardlink, verbose, preserveMTime)
return nil
})
}
Expand All @@ -83,7 +84,7 @@ func main() {
args := os.Args[1:]

if len(args) < 2 {
fmt.Println("Usage: copy_directory src dst [--hardlink] [--verbose]")
fmt.Println("Usage: copy_directory src dst [--hardlink] [--verbose] [--preserve-mtime]")
os.Exit(1)
}

Expand All @@ -96,6 +97,8 @@ func main() {
hardlink = true
} else if a == "--verbose" {
verbose = true
} else if a == "--preserve-mtime" {
preserveMTime = true
}
}
}
Expand Down
5 changes: 3 additions & 2 deletions tools/copy_to_directory/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ type config struct {
IncludeSrcsPatterns []string `json:"include_srcs_patterns"`
ReplacePrefixes map[string]string `json:"replace_prefixes"`
RootPaths []string `json:"root_paths"`
PreserveMTime bool `json:"preserve_mtime"`
Verbose bool `json:"verbose"`

ReplacePrefixesKeys []string
Expand Down Expand Up @@ -317,7 +318,7 @@ func (w *walker) copyPath(cfg *config, file fileInfo, outputPath string) error {

if !cfg.AllowOverwrites {
// if we don't allow overwrites then we can start copying as soon as a copy is calculated
w.queue <- common.NewCopyOpts(file.Path, outputPath, file.FileInfo, file.Hardlink, cfg.Verbose)
w.queue <- common.NewCopyOpts(file.Path, outputPath, file.FileInfo, file.Hardlink, cfg.Verbose, cfg.PreserveMTime)
}

return nil
Expand Down Expand Up @@ -419,7 +420,7 @@ func main() {
// if we allow overwrites then we must wait until all copy paths are calculated before starting
// any copy operations
for outputPath, file := range copySet {
queue <- common.NewCopyOpts(file.Path, outputPath, file.FileInfo, file.Hardlink, cfg.Verbose)
queue <- common.NewCopyOpts(file.Path, outputPath, file.FileInfo, file.Hardlink, cfg.Verbose, cfg.PreserveMTime)
}
}

Expand Down
Loading