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

use containerd's epoch package for handling SOURCE_DATE_EPOCH #1908

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

thaJeztah
Copy link
Member

This allows us to validate the value before we're using it, and makes sure we handle things in the same way.

This allows us to validate the value before we're using it, and makes
sure we handle things in the same way.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Comment on lines +125 to +130
v, err := epoch.SourceDateEpoch()
if err != nil {
return nil, err
}
if v != nil {
opts.BuildArgs[epoch.SourceDateEpochEnv] = strconv.FormatInt(v.Unix(), 10)
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure parsing string to time and back to string for this purpose is ideal.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, it's not; I started working on a branch to add a utility for parsing in containerd (although we have one in BuildKit as well).

Copy link
Member Author

Choose a reason for hiding this comment

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

I could still use the parsing, and just get the env-var separately; let me check for a bit

Copy link
Member Author

Choose a reason for hiding this comment

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

hm.. no no utility on its own, so if could be;

opts.BuildArgs[epoch.SourceDateEpochEnv] = os.Getenv(epoch.SourceDateEpochEnv)

which also looks a bit odd, as we're setting a value that's not the v != nil that we check 😞

@tonistiigi
Copy link
Member

@thaJeztah What's the status of this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants