-
Notifications
You must be signed in to change notification settings - Fork 130
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
Remove cobra #54
Comments
I forgot. cobra gets pulled in anyway because cosign depends on it. Removing it won't help anything. |
Agree with the motivation here, we should try and reduce build time for the slsa-github-generator binaries. In my experimentation I have projects which take about ~30s to build but building One thing that will likely help here is cosign's efforts to reduce their dependency tree and factor out more of their reusable logic into sigstore/sigstore, see sigstore/cosign#1462 |
Yeah, right now our direct dependencies are small(ish) but the cosign cli packages import a lot of stuff (including cobra and tools for parsing flags etc.) that we don't technically need so the indirect dependencies are pretty crazy. I think the major part of the 2min compilation is downloading the dependencies. However, even if we remove cobra it's in the cosign cli packages we use so it will still get pulled by our CI ( cobra is particularly bad with regard to indirect dependencies (it's kind of infamous at this point). It brings in stuff like etcd libraries, mongodb drivers, runc, ... (though, I suspect most don't actually make it into the binary). |
@joshuagl Could you create an issue specifically about build time? maybe we can think about and address it at some point. |
Filed as #74 and changed to generic title of "reducing action runtime" |
github.com/spf13/cobra is nice and all but the amount of dependencies it brings in is pretty ridiculous.
Maybe we can consider using something more reasonable like https://github.com/google/subcommands ?
The text was updated successfully, but these errors were encountered: