Skip to content

content/local: inline sys.StatATimeAsTime()#5633

Merged
estesp merged 1 commit intocontainerd:mainfrom
thaJeztah:inline_statatimeastime
Jun 22, 2021
Merged

content/local: inline sys.StatATimeAsTime()#5633
estesp merged 1 commit intocontainerd:mainfrom
thaJeztah:inline_statatimeastime

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

relates to docker/cli#3154

The sys.StatATimeAsTime() utility is currently only used in a single place, but because it's living in the "sys" package, also brings in other dependencies, such as Microsoft/hcsshim.

This patch inlines the code from sys.StatATimeAsTime(), to remove that dependency.

We can consider removing the utilities in sys in favor of their copy in containerd/continuity;

// StatAtime returns the Atim
func StatAtime(st *syscall.Stat_t) syscall.Timespec {
return st.Atim
}
// StatCtime returns the Ctim
func StatCtime(st *syscall.Stat_t) syscall.Timespec {
return st.Ctim
}
// StatMtime returns the Mtim
func StatMtime(st *syscall.Stat_t) syscall.Timespec {
return st.Mtim
}
// StatATimeAsTime returns st.Atim as a time.Time
func StatATimeAsTime(st *syscall.Stat_t) time.Time {
return time.Unix(int64(st.Atim.Sec), int64(st.Atim.Nsec)) // nolint: unconvert
}

https://github.com/containerd/continuity/blob/33a84563724b68f70fc71fbb632cf9e0ad60ba5d/fs/stat_linuxopenbsd.go#L26-L45

(I'll have a look at that in a follow-up)

The sys.StatATimeAsTime() utility is currently only used in a single place,
but because it's living in the "sys" package, also brings in other dependencies,
such as Microsoft/hcsshim.

This patch inlines the code from sys.StatATimeAsTime(), to remove that dependency.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah
Copy link
Copy Markdown
Member Author

@AkihiroSuda @samuelkarp @estesp PTAL

I'd like to cherry-pick this into the 1.5 release branch, as it's a minor refactor and (see docker/cli#3154) makes a big difference on the dependency-tree in our CLI (assuming we want to stick to consuming the releases / release-branches of containerd in the code there)

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Jun 22, 2021

Build succeeded.

@thaJeztah
Copy link
Copy Markdown
Member Author

opened #5634 as draft

Copy link
Copy Markdown
Member

@fuweid fuweid left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

@estesp estesp merged commit ddeacc4 into containerd:main Jun 22, 2021
@thaJeztah thaJeztah deleted the inline_statatimeastime branch June 22, 2021 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cherry-picked/1.5.x PR commits are cherry-picked into release/1.5 branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants