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

code-generator is missing from this repo #6

Closed
0xmichalis opened this issue Jan 19, 2018 · 13 comments
Closed

code-generator is missing from this repo #6

0xmichalis opened this issue Jan 19, 2018 · 13 comments

Comments

@0xmichalis
Copy link
Contributor

  1. git cloned this repo
  2. changed the types to my own
  3. removed all generated files (rm -rf pkg/client pkg/apis/prow/v1/zz_generated.deepcopy.go)
  4. ./hack/update-codegen.sh does not work because code-generator is not vendored in this repo
  5. pushd vendor/k8s.io
  6. git clone https://github.com/kubernetes/code-generator
  7. popd; ./hack/update-codegen.sh now works

I didn't expect to do 5-7. I assumed that since this is a standalone repo, code generation should work ootb. Intentional?

@munnerz @sttts

@sttts
Copy link
Contributor

sttts commented Jan 19, 2018

Intentional?

Intentional? no. Known? yes. Godeps does not do it itself and the publisher does not fix it up (but could).

@munnerz
Copy link
Member

munnerz commented Jan 19, 2018

Yep this is a bit of a tricky one right now - does Godeps support explicitly vendoring a package that the project doesn't actually depend upon?

Even if we make the publisher do this, we need to make sure a godeps save ./... in future doesn't wipe out the code-generator dependency (thinking about end-users who might copy and paste this repo to start their own)

@sttts
Copy link
Contributor

sttts commented Jan 19, 2018

(thinking about end-users who might copy and paste this repo to start their own)

An hack/update-godeps.sh could help I think. But I wonder whether we really have to protect the users from the vendoring tools so much. The published repo should of course work. But if users copy the code, they have to know their vendoring tool of choice. And yes, these are a pain in Goland.

@0xmichalis
Copy link
Contributor Author

Another option is to create a dependency link between the generator and the sample repo with an empty import. Could that work?

import _ "k8s.io/code-generator"

@0xmichalis
Copy link
Contributor Author

Or hack/update-codegen.sh can ensure the generator is cloned in the directory it expects to find it.

@sttts
Copy link
Contributor

sttts commented Jan 19, 2018

Or hack/update-codegen.sh can ensure the generator is cloned in the directory it expects to find it.

Which means it must be aware of its branch and pick the right version. Don't like that either. We need a sane vendoring tool like dep that can reference extra repos.

@0xmichalis
Copy link
Contributor Author

/meow

@k8s-ci-robot
Copy link
Contributor

@Kargakis: cat image

In response to this:

/meow

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@0xmichalis
Copy link
Contributor Author

0xmichalis commented Feb 4, 2018

Which means it must be aware of its branch and pick the right version. Don't like that either. We need a sane vendoring tool like dep that can reference extra repos.

@sttts today code-generator and sample-controller follow the release branches of kubernetes/kubernetes. Unless that is going to change in the future, I think hack/update-codegen.sh should try to checkout the same branch in code-generatorthat is already checkout out in sample-controller when the script runs. Seems like a viable short-term fix, dep is unlike to be used anytime soon.

@sttts
Copy link
Contributor

sttts commented Feb 4, 2018

dep is unlike to be used anytime soon.

You might be surprised soon ;)

@carolynvs
Copy link

I am using dep to get started on a fork of this repo. Here's my config for vendoring the code generator (relies on dep v0.4.1+):

required = [
  "k8s.io/code-generator/cmd/client-gen"
]

[[constraint]]
  name = "k8s.io/client-go"
  version = "kubernetes-1.9.2"

[[constraint]]
  name = "k8s.io/api"
  version = "kubernetes-1.9.2"

[[constraint]]
  name = "k8s.io/apimachinery"
  version = "kubernetes-1.9.2"

[[constraint]]
  name = "k8s.io/code-generator"
  version = "kubernetes-1.9.2"

[prune]
  non-go = true
  go-tests = true
  unused-packages = true

  [[prune.project]]
    name = "k8s.io/code-generator"
    unused-packages = false
    non-go = false
    go-tests = false

  [[prune.project]]
    name = "k8s.io/gengo"
    unused-packages = false
    non-go = false
    go-tests = false

I haven't gotten to the point where everything is working so that may need to change some more. 😀

@sttts
Copy link
Contributor

sttts commented Feb 8, 2018

I haven't gotten to the point where everything is working so that may need to change some more.

What is still missing?

Am surprised that we have to disable pruning for required packages, at least the unused-packages = false part.

@carolynvs
Copy link

What I posted is a bit of a jenky hack to workaround golang/dep#1545 not being merged yet.

  • The root of code-generator package doesn't contain go code, so dep is currently not letting me require it. As a workaround I am requiring a subpackage (cmd/codegen).
  • The extra project level prune settings tell dep to not remove the root package contents. Dep thinks the root project is unused because I wasn't able to require the root project.

This is a temporary wart in dep hopefully. Long term, just the required should be sufficient.

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

No branches or pull requests

5 participants