-
Notifications
You must be signed in to change notification settings - Fork 44
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
[WSL-369] Cache rootfs every night #345
Conversation
d1acabb
to
7cace97
Compare
"errors" | ||
"os" | ||
|
||
"github.com/0xrawsec/golang-utils/log" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This import seems suspicious. Are you sure it is what you intended it to be? I don't see a lot of activity in that repository.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough, I replaced it with Fprintf
since we don't really need a log for such small executable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think we don’t really need log here. Once this proposal is settled (https://go.googlesource.com/proposal/+/master/design/56345-structured-logging.md), we will be able to standardize around it for all our packages and binaries.
9afd9cf
to
d9c1d96
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is it for a first round. There will need to be another review as I think the code is going to change a bit with those suggestions :)
Quite the review haha. I applied most of the changes you suggested and made the rest obsolete by simplifying the program. I was being fancy with a degraded output but there really is no need for that complexity. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One minor concern about the imports.
14044b7
to
d03d8da
Compare
Piping on Windows doesn't work because .\lp-distro-info > $file stores it UTF-16 encoded with the BOM. This breaks the csv package in Go, which is used to parse this program's output. The solution is to implement: lp-distro-info -o $file where the default output is stdout to maintain backwards-compatibility.
Some code had to be factored out of prepare-build so as to have a single implementation of the rootfs URL generator.
Also ran the unit tests here to ensure I did not break it: https://github.com/EduardGomezEscandell/AdventOfCode/blob/main/2022/utils/array/array_test.go#L415-L448
d03d8da
to
0f58385
Compare
Rootfs download is an expensive part of the E2E CI run. This PR sets a cron job that will download the rootfs every night.
e2e
workflows will be able to use this.To avoid having yet another place that we need to update every time a new release comes out, some parts of
wsl-builder/prepare-build/buildGHMatrix.go
were extracted into thecommon
package, and a new executablerelease-info
is built to access this data:Example output: