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 default tmpdir for temporary files #13

Closed
wants to merge 1 commit into from

Conversation

ekohl
Copy link
Member

@ekohl ekohl commented Jun 26, 2023

This replaces the (as of Go 1.17) deprecated ioutil.TempFile with os.CreateTemp. When calling os.CreateTemp with an empty dir the default directory for temporary files is used. On UNIX that's $TMPDIR if non-empty or /tmp.

@ekohl
Copy link
Member Author

ekohl commented Jun 26, 2023

@adamruzicka this is my first patch in Go. I don't know what the convention is when supporting older versions. I'm guessing Go 1.15 is still needed for EL7 support.

@adamruzicka
Copy link
Contributor

I don't know what the convention is when supporting older versions

I'm not really a go person, but I'm afraid there is none.

Go 1.15 is still needed for EL7 support.

Well, yes and no. As far as I know the go compiler creates static binaries so we could build on newer EL and ship the resulting artifacts on older ELs but I guess this would mean we'd have to deviate quite a bit from how we normally package things.

@ekohl
Copy link
Member Author

ekohl commented Jun 26, 2023

I wonder how we're consuming golang. EPEL7 has golang 1.19.9 and EL8 has golang 1.20.4 so do we really need to support older versions? Or is it for downstream?

@adamruzicka
Copy link
Contributor

As far as I know in upstream we take it from epel, which back then only had 1.15. In downstream go seems to be added to the buildroot, in july last year it had 1.16. It sounds like it should be safe to drop 1.15.

@adamruzicka
Copy link
Contributor

Now that I'm thinking about it, I'm not sure if this would actually help. The worker is spawned by yggdrasil and yggdrasil trims down the environment substantially before starting the worker. I'm not sure if TEMPDIR survives this purge.

@ekohl
Copy link
Member Author

ekohl commented Jun 26, 2023

That's a fair point, but it still would be better to avoid hardcoding a path. For now to see if it builds I added a commit that updates the matrix.

@ekohl
Copy link
Member Author

ekohl commented Jun 26, 2023

That looks green so I opened #14.

This replaces the (as of Go 1.17) deprecated ioutil.TempFile with
os.CreateTemp. When calling os.CreateTemp with an empty dir the default
directory for temporary files is used. On UNIX that's $TMPDIR if
non-empty or /tmp.
@ekohl
Copy link
Member Author

ekohl commented Jun 26, 2023

Rebased

@ekohl
Copy link
Member Author

ekohl commented Jul 28, 2023

@adamruzicka any progress on this?

@adamruzicka
Copy link
Contributor

adamruzicka commented Sep 21, 2023

Included in #16 at 1bc71c7, thank you @ekohl !

@ekohl ekohl deleted the use-default-tmpdir branch September 21, 2023 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants