TEST: update codecgen #16559

mikedanese · 2015-10-29T23:02:31Z

Running ./hack/update-codecgen.sh on master.

Is it out of date or am I doing something wrong?

mikedanese · 2015-10-29T23:09:26Z

Appearantly I am wrong and everyone else is right.

k8s-bot · 2015-10-29T23:29:49Z

GCE e2e test build/test passed for commit 6c49ab1.

wojtek-t · 2015-10-30T07:07:25Z

@mikedanese - what was your problem? Maybe we can improve this script so that you will not face the same problem again?

mikedanese · 2015-11-06T04:14:51Z

thanks @wojtek-t, i'll file an issue. I think #16877 is running into this as well. The tranformation in that diff is almost exactly the same as this.

anish · 2015-11-06T04:33:34Z

Yep, I authored #16877 pretty much the same issue. Would be nice to not have my commits change from size/M to size/XXL. Perhaps #16128 is related ?

wojtek-t · 2015-11-06T07:17:18Z

@mikedanese - are you synced to head?
I fixed hack/update-codecgen script in #16788 ~yesterday.

anish · 2015-11-06T07:46:07Z

@wojtek-t I'm definitely synced up to head, but still see the same issue as @mikedanese

wojtek-t · 2015-11-06T08:06:49Z

@anish - #16128 is definitely responsible for making your PR xxl - but that's a completely separate issue

I took a look into your PR and I looks exactly as the problem that we solved with #16788
Did you run hack/update-codecgen after syncing to head? If so, can you please paste its output here?

anish · 2015-11-06T08:25:24Z

@wojtek-t

anish@hubland kubernetes > git fetch upstream
anish@hubland kubernetes > git rebase upstream/master
Current branch master is up to date.
anish@hubland kubernetes > git checkout iscsi_iface
Switched to branch 'iscsi_iface'
anish@hubland kubernetes > git rebase master
Current branch iscsi_iface is up to date.
anish@hubland kubernetes > hack/update-codecgen.sh 
./pkg/storage/testing/types.generated.go is regenerated.
./pkg/apis/metrics/v1alpha1/types.generated.go is regenerated.
./pkg/apis/metrics/types.generated.go is regenerated.
./pkg/apis/componentconfig/v1alpha1/types.generated.go is regenerated.
./pkg/apis/componentconfig/types.generated.go is regenerated.
./pkg/apis/extensions/v1beta1/types.generated.go is regenerated.
./pkg/apis/extensions/types.generated.go is regenerated.
./pkg/apiserver/testing/types.generated.go is regenerated.
./pkg/kubectl/testing/types.generated.go is regenerated.
./pkg/api/v1/types.generated.go is regenerated.
./pkg/api/types.generated.go is regenerated.
anish@hubland kubernetes > git status
On branch iscsi_iface
nothing to commit, working directory clean
anish@hubland kubernetes > git push -f origin iscsi_iface

is what is resulting in the failure for me

wojtek-t · 2015-11-06T08:33:59Z

@anish hmmm
the order in which the script is regenerating files is clearly wrong - this is what my PR was intended to fix

Can you please add the following two lines two your script:

echo ${index[@]}
echo ${generated_files[@]}

right after index is computed:
https://github.com/kubernetes/kubernetes/blob/master/hack/update-codecgen.sh#L82

and paste the output again
[sorry - I'm not able to reproduce this problem locally].

anish · 2015-11-06T08:36:39Z

@wojtek-t

anish@hubland kubernetes > hack/update-codecgen.sh 
0 1 2 3 4 5 6 7 8 9 10
./pkg/storage/testing/types.generated.go ./pkg/apis/metrics/v1alpha1/types.generated.go ./pkg/apis/metrics/types.generated.go ./pkg/apis/componentconfig/v1alpha1/types.generated.go ./pkg/apis/componentconfig/types.generated.go ./pkg/apis/extensions/v1beta1/types.generated.go ./pkg/apis/extensions/types.generated.go ./pkg/apiserver/testing/types.generated.go ./pkg/kubectl/testing/types.generated.go ./pkg/api/v1/types.generated.go ./pkg/api/types.generated.go
./pkg/storage/testing/types.generated.go is regenerated.
./pkg/apis/metrics/v1alpha1/types.generated.go is regenerated.
./pkg/apis/metrics/types.generated.go is regenerated.
./pkg/apis/componentconfig/v1alpha1/types.generated.go is regenerated.
./pkg/apis/componentconfig/types.generated.go is regenerated.
./pkg/apis/extensions/v1beta1/types.generated.go is regenerated.
./pkg/apis/extensions/types.generated.go is regenerated.
./pkg/apiserver/testing/types.generated.go is regenerated.
./pkg/kubectl/testing/types.generated.go is regenerated.
./pkg/api/v1/types.generated.go is regenerated.
./pkg/api/types.generated.go is regenerated.

wojtek-t · 2015-11-06T08:49:19Z

@anish
Thanks - this is clearly not what it is expected to be (it's definitely not topologically sorted).
Can you please also add the following line:

echo "$1" ${j} $(depends "$1" ${j}); then

inside if !visited if in tsort function:
https://github.com/kubernetes/kubernetes/blob/master/hack/update-codecgen.sh#L69

anish · 2015-11-06T08:56:03Z

@wojtek-t
To double check, this is the diff you wanted :

     if ! ${visited[${j}]}; then
-      if $(depends "$1" ${j}); then
+      if echo "$1" ${j} $(depends "$1" ${j}); then
         tsort $j

anish@hubland kubernetes > hack/update-codecgen.sh 
0 1 false
1 2 false
2 3 false
3 4 false
4 5 false
5 6 false
6 7 false
7 8 false
8 9 false
9 10 false
10 9 8 7 6 5 4 3 2 1 0
./pkg/storage/testing/types.generated.go ./pkg/apis/metrics/v1alpha1/types.generated.go ./pkg/apis/metrics/types.generated.go ./pkg/apis/componentconfig/v1alpha1/types.generated.go ./pkg/apis/componentconfig/types.generated.go ./pkg/apis/extensions/v1beta1/types.generated.go ./pkg/apis/extensions/types.generated.go ./pkg/apiserver/testing/types.generated.go ./pkg/kubectl/testing/types.generated.go ./pkg/api/v1/types.generated.go ./pkg/api/types.generated.go
./pkg/api/types.generated.go is regenerated.
./pkg/api/v1/types.generated.go is regenerated.
./pkg/kubectl/testing/types.generated.go is regenerated.
./pkg/apiserver/testing/types.generated.go is regenerated.
./pkg/apis/extensions/types.generated.go is regenerated.
./pkg/apis/extensions/v1beta1/types.generated.go is regenerated.
./pkg/apis/componentconfig/types.generated.go is regenerated.
./pkg/apis/componentconfig/v1alpha1/types.generated.go is regenerated.
./pkg/apis/metrics/types.generated.go is regenerated.
./pkg/apis/metrics/v1alpha1/types.generated.go is regenerated.
./pkg/storage/testing/types.generated.go is regenerated.

wojtek-t · 2015-11-06T09:04:56Z

Sorry for not being clear enough.
I just wanted to add this line, not replace, i.e.:

if ! ${visited[${j}]}; then
+     echo "$1" ${j} $(depends "$1" ${j})
      if $(depends "$1" ${j}); then
         tsort $j

The change you made significantly changes the logic - that's why it changed a lot.

anish · 2015-11-06T09:09:16Z

aah, that's what I thought but the dangling then threw me off :)

anish@hubland kubernetes >  hack/update-codecgen.sh    
0 1 false
0 2 false
0 3 false
0 4 false
0 5 false
0 6 false
0 7 false
0 8 false
0 9 false
0 10 false
1 2 false
1 3 false
1 4 false
1 5 false
1 6 false
1 7 false
1 8 false
1 9 false
1 10 false
2 3 false
2 4 false
2 5 false
2 6 false
2 7 false
2 8 false
2 9 false
2 10 false
3 4 false
3 5 false
3 6 false
3 7 false
3 8 false
3 9 false
3 10 false
4 5 false
4 6 false
4 7 false
4 8 false
4 9 false
4 10 false
5 6 false
5 7 false
5 8 false
5 9 false
5 10 false
6 7 false
6 8 false
6 9 false
6 10 false
7 8 false
7 9 false
7 10 false
8 9 false
8 10 false
9 10 false
0 1 2 3 4 5 6 7 8 9 10
./pkg/storage/testing/types.generated.go ./pkg/apis/metrics/v1alpha1/types.generated.go ./pkg/apis/metrics/types.generated.go ./pkg/apis/componentconfig/v1alpha1/types.generated.go ./pkg/apis/componentconfig/types.generated.go ./pkg/apis/extensions/v1beta1/types.generated.go ./pkg/apis/extensions/types.generated.go ./pkg/apiserver/testing/types.generated.go ./pkg/kubectl/testing/types.generated.go ./pkg/api/v1/types.generated.go ./pkg/api/types.generated.go
./pkg/storage/testing/types.generated.go is regenerated.
./pkg/apis/metrics/v1alpha1/types.generated.go is regenerated.
./pkg/apis/metrics/types.generated.go is regenerated.
./pkg/apis/componentconfig/v1alpha1/types.generated.go is regenerated.
./pkg/apis/componentconfig/types.generated.go is regenerated.
./pkg/apis/extensions/v1beta1/types.generated.go is regenerated.
./pkg/apis/extensions/types.generated.go is regenerated.
./pkg/apiserver/testing/types.generated.go is regenerated.
./pkg/kubectl/testing/types.generated.go is regenerated.
./pkg/api/v1/types.generated.go is regenerated.
./pkg/api/types.generated.go is regenerated.

wojtek-t · 2015-11-06T09:13:12Z

@anish
Awesome - thanks. So it's clear now that "depends" function doesn't work in your case.

So one more experiment - can you please add:

echo ${candidate}
echo ${deps[@]}

in depends function:
https://github.com/kubernetes/kubernetes/blob/master/hack/update-codecgen.sh#L55

anish · 2015-11-06T09:18:29Z

with

   candidate=$(dirname "${fullpath}")
+  echo ${candidate}
+  echo ${deps[@]}
   result=false

I get this output : https://gist.github.com/anish/975a611a503323d74ab6

wojtek-t · 2015-11-06T09:33:56Z

@anish I think I know where the problem is.

The script is assuming that you have the repository under:

*/k8s.io/kubernetes

directory. It seems it's not the case for you.

Am I right?

wojtek-t · 2015-11-06T09:34:50Z

So to be clear - I have my repository under:

/usr/local/google/home/wojtekt/go/src/k8s.io/kubernetes

This is matching "*/k8s.io/kubernetes"

anish · 2015-11-06T09:38:40Z

Yes, I don't have that path. Do you need the output with the path changed or should I just try the build with the path fixed ?

wojtek-t · 2015-11-06T09:40:39Z

@anish - you can try fixing the path and I'm pretty sure this will solve the problem. Let me know if that will not help you.

anish · 2015-11-06T09:41:58Z

thanks @wojtek-t ! should know if it's fixed in about 10 min or so

anish · 2015-11-06T10:00:07Z

@wojtek-t Nope, same issue still : https://api.travis-ci.org/jobs/89610431/log.txt?deansi=true

Actual moving the directories and re-running showed no git changes at all

anish@hubland kubernetes > pwd
/dump/workspace/k8s.io/kubernetes

anish@hubland kubernetes > hack/update-codecgen.sh > /tmp/gist 2>&1 

anish@hubland kubernetes > git status
On branch iscsi_iface
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

        modified:   hack/update-codecgen.sh

no changes added to commit (use "git add" and/or "git commit -a")

gist : https://gist.github.com/anish/2efdb24accb522c0d063

wojtek-t · 2015-11-06T10:08:50Z

@anish
Can you please paste one more what you are printing (e.g. diff of hack/update-codecgen.sh)?

anish · 2015-11-06T10:11:03Z

Sure :

diff --git a/hack/update-codecgen.sh b/hack/update-codecgen.sh
index 63f1fa5..2271429 100755
--- a/hack/update-codecgen.sh
+++ b/hack/update-codecgen.sh
@@ -53,6 +53,8 @@ function depends {
   deps=$(go list -f "{{.Deps}}" ${file} | tr "[" " " | tr "]" " ")
   fullpath=$(readlink -f ${generated_files[$2]//\.generated\.go/.go})
   candidate=$(dirname "${fullpath}")
+  echo ${candidate}
+  echo ${deps[@]}
   result=false
   for dep in ${deps}; do
     if [[ ${candidate} = *${dep} ]]; then
@@ -67,6 +69,7 @@ function tsort {
   local j=0
   for (( j=0; j<number; j++ )); do
     if ! ${visited[${j}]}; then
+      echo "$1" ${j} $(depends "$1" ${j})
       if $(depends "$1" ${j}); then
         tsort $j
       fi
@@ -80,6 +83,8 @@ for (( i=0; i<number; i++ )); do
   fi
 done
 index=(${result})
+echo ${index[@]}
+echo ${generated_files[@]}

 CODECGEN="${PWD}/codecgen_binary"
 godep go build -o "${CODECGEN}" github.com/ugorji/go/codec/codecgen

wojtek-t · 2015-11-06T10:21:56Z

@anish - sorry I have no idea what is going on here.

Can you please:

replace the line:

echo "$1" ${j} $(depends "$1" ${j})

with just

echo "$1" ${j}

remove lines
```
echo ${candidate}
echo $(deps[@]}
```

add the following line:

echo "${candidate}" "${dep}" "$(${candidate} = *${dep})"

just after

    for dep in ${deps}; do

anish · 2015-11-06T10:33:05Z

@wojtek-t

--- a/hack/update-codecgen.sh
+++ b/hack/update-codecgen.sh
@@ -55,6 +55,7 @@ function depends {
   candidate=$(dirname "${fullpath}")
   result=false
   for dep in ${deps}; do
+    echo "${candidate}" "${dep}" "$(${candidate} = *${dep})"
     if [[ ${candidate} = *${dep} ]]; then
       result=true
     fi
@@ -67,6 +68,7 @@ function tsort {
   local j=0
   for (( j=0; j<number; j++ )); do
     if ! ${visited[${j}]}; then
+      echo "$1" ${j}
       if $(depends "$1" ${j}); then
         tsort $j
       fi
@@ -80,6 +82,8 @@ for (( i=0; i<number; i++ )); do
   fi
 done
 index=(${result})
+echo ${index[@]}
+echo ${generated_files[@]}

 CODECGEN="${PWD}/codecgen_binary"

output at https://gist.github.com/anish/9e2649704fdffaa0cdd8

wojtek-t · 2015-11-06T10:38:46Z

@anish

Sorry - I think that if you have any "echo" inside "depends" function, then it will affect the result of that function. Can you please remove all "echo" instructions from "depends" function?

anish · 2015-11-06T10:44:31Z

@wojtek-t

--- a/hack/update-codecgen.sh
+++ b/hack/update-codecgen.sh
@@ -67,6 +67,7 @@ function tsort {
   local j=0
   for (( j=0; j<number; j++ )); do
     if ! ${visited[${j}]}; then
+      echo "$1" ${j}
       if $(depends "$1" ${j}); then
         tsort $j
       fi
@@ -80,6 +81,8 @@ for (( i=0; i<number; i++ )); do
   fi
 done
 index=(${result})
+echo ${index[@]}
+echo ${generated_files[@]}

 CODECGEN="${PWD}/codecgen_binary"
 godep go build -o "${CODECGEN}" github.com/ugorji/go/codec/codecgen

https://gist.github.com/anish/ded6190ab950057d486f

This also resulted in change to the files :

        modified:   hack/update-codecgen.sh
        modified:   pkg/apis/extensions/types.generated.go
        modified:   pkg/apis/extensions/v1beta1/types.generated.go
        modified:   pkg/apiserver/testing/types.generated.go
        modified:   pkg/kubectl/testing/types.generated.go
        modified:   pkg/storage/testing/types.generated.go

no changes added to commit (use "git add" and/or "git commit -a")

anish@hubland kubernetes > git diff | wc -l
37281

wojtek-t · 2015-11-06T10:46:18Z

@anish - so I think this is what you expected right?

If you run hack/verify-codecgen.sh, this should work now, right?

anish · 2015-11-06T10:47:53Z

@wojtek-t I believe so, trying it out now

anish · 2015-11-06T10:59:21Z

@wojtek-t that was it, thanks ! build is passing now

mikedanese · 2015-11-06T17:39:06Z

@wojtek-t @anish thanks for debugging this!

wojtek-t · 2015-11-06T17:40:51Z

@mikedanese - I will add a comment to the file, but I spent the whole day debugging a different issue today.

mikedanese · 2015-11-06T17:41:47Z

No worries, i believe this is somewhat lower priority.

anish · 2015-11-06T21:48:06Z

@mikedanese thanks for actually pointing me to this, @wojtek-t deserves all the credit :)

wojtek-t · 2015-11-06T21:50:46Z

@anish - thanks for your patience - this wasn't obvious

update codecgen

6c49ab1

googlebot added the cla: yes label Oct 29, 2015

mikedanese assigned wojtek-t Oct 29, 2015

mikedanese closed this Oct 29, 2015

mikedanese deleted the codecgen branch November 6, 2015 21:54

wojtek-t mentioned this pull request Nov 9, 2015

Update comment in hack/update-codecgen.sh script #17005

Merged

TEST: update codecgen #16559

TEST: update codecgen #16559

Conversation

mikedanese commented Oct 29, 2015

mikedanese commented Oct 29, 2015

k8s-bot commented Oct 29, 2015

wojtek-t commented Oct 30, 2015

mikedanese commented Nov 6, 2015

anish commented Nov 6, 2015

wojtek-t commented Nov 6, 2015

anish commented Nov 6, 2015

wojtek-t commented Nov 6, 2015

anish commented Nov 6, 2015

wojtek-t commented Nov 6, 2015

anish commented Nov 6, 2015

wojtek-t commented Nov 6, 2015

anish commented Nov 6, 2015

wojtek-t commented Nov 6, 2015

anish commented Nov 6, 2015

wojtek-t commented Nov 6, 2015

anish commented Nov 6, 2015

wojtek-t commented Nov 6, 2015

wojtek-t commented Nov 6, 2015

anish commented Nov 6, 2015

wojtek-t commented Nov 6, 2015

anish commented Nov 6, 2015

anish commented Nov 6, 2015

wojtek-t commented Nov 6, 2015

anish commented Nov 6, 2015

wojtek-t commented Nov 6, 2015

anish commented Nov 6, 2015

wojtek-t commented Nov 6, 2015

anish commented Nov 6, 2015

wojtek-t commented Nov 6, 2015

anish commented Nov 6, 2015

anish commented Nov 6, 2015

mikedanese commented Nov 6, 2015

wojtek-t commented Nov 6, 2015

mikedanese commented Nov 6, 2015

anish commented Nov 6, 2015

wojtek-t commented Nov 6, 2015