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

Cleanup vendor #5726

Merged
merged 7 commits into from
Jan 29, 2018
Merged

Cleanup vendor #5726

merged 7 commits into from
Jan 29, 2018

Conversation

im-kulikov
Copy link
Contributor

Docker build successfully:

   Sending build context to Docker daemon   84.8MB
   Step 1/10 : FROM golang:1.9-alpine as builder
   1.9-alpine: Pulling from library/golang
   605ce1bd3f31: Pull complete
   d1e3a1512603: Pull complete
   64455c1aaa47: Pull complete
   cdc57526dc05: Pull complete
   92b355b184ea: Pull complete
   5c5463f1346a: Pull complete
   Digest: sha256:42a5e73c830a2379e8f75b195cd53e43cb1c221ac1d75b91265ef0974de6482a
   Status: Downloaded newer image for golang:1.9-alpine
    ---> a4c7ad5b9041
   Step 2/10 : RUN apk add --no-cache     make     git
    ---> Running in 400795a9e773
   fetch http://dl-cdn.alpinelinux.org/alpine/v3.6/main/x86_64/APKINDEX.tar.gz
   fetch http://dl-cdn.alpinelinux.org/alpine/v3.6/community/x86_64/APKINDEX.tar.gz
   (1/6) Installing libssh2 (1.8.0-r1)
   (2/6) Installing libcurl (7.57.0-r0)
   (3/6) Installing expat (2.2.0-r1)
   (4/6) Installing pcre (8.41-r0)
   (5/6) Installing git (2.13.5-r0)
   (6/6) Installing make (4.2.1-r0)
   Executing busybox-1.26.2-r9.trigger
   OK: 25 MiB in 18 packages
   Removing intermediate container 400795a9e773
    ---> 02ad96bc3da0
   Step 3/10 : COPY . /go/src/github.com/pingcap/tidb
    ---> 08fa94416742
   Step 4/10 : WORKDIR /go/src/github.com/pingcap/tidb/
   Removing intermediate container a850a8ead739
    ---> d8b1af371946
   Step 5/10 : RUN make
    ---> Running in 4c92bc149091
   make parser
   make[1]: Entering directory '/go/src/github.com/pingcap/tidb'
   CGO_ENABLED=0 go build  -o bin/goyacc parser/goyacc/main.go
   bin/goyacc -o /dev/null parser/parser.y
   Parse table entries: 611181 of 1601340, x 16 bits == 1222362 bytes
   bin/goyacc -o parser/parser.go parser/parser.y 2>&1 | egrep "(shift|reduce)/reduce" | awk '{print} END {if (NR > 0) {print "Find conflict in parser.y. Please check y.output for more information."; exit 1;}}'
   rm -f y.output
   make[1]: Leaving directory '/go/src/github.com/pingcap/tidb'
   CGO_ENABLED=0 go build   -ldflags '-X "github.com/pingcap/tidb/mysql.TiDBReleaseVersion=v1.1.0-alpha.1-24-g96bd4e11-dirty" -X "github.com/pingcap/tidb/util/printer.TiDBBuildTS=2018-01-26 11:57:54" -X "github.com/pingcap/tidb/util/printer.TiDBGitHash=96bd4e11621665ec40934b35f1cee7b77aa8a851" -X "github.com/pingcap/tidb/util/printer.TiDBGitBranch=master" -X "github.com/pingcap/tidb/util/printer.GoVersion=go version go1.9.3 linux/amd64"' -o bin/tidb-server tidb-server/main.go
   Build TiDB Server successfully!
   Removing intermediate container 4c92bc149091
    ---> 16da7786d02c
   Step 6/10 : FROM scratch
    --->
   Step 7/10 : COPY --from=builder /go/src/github.com/pingcap/tidb/bin/tidb-server /tidb-server
    ---> 455347e6b0c2
   Step 8/10 : WORKDIR /
   Removing intermediate container 3f4572ccf0bc
    ---> 01bf74f5c71f
   Step 9/10 : EXPOSE 4000
    ---> Running in 23eaa5cc2adc
   Removing intermediate container 23eaa5cc2adc
    ---> fc91019cc0a5
   Step 10/10 : ENTRYPOINT ["/tidb-server"]
    ---> Running in 0773ce0ba0a1
   Removing intermediate container 0773ce0ba0a1
    ---> aaa9c01eaaf9
   Successfully built aaa9c01eaaf9

Docker build successfully:
```docker build .
   Sending build context to Docker daemon   84.8MB
   Step 1/10 : FROM golang:1.9-alpine as builder
   1.9-alpine: Pulling from library/golang
   605ce1bd3f31: Pull complete
   d1e3a1512603: Pull complete
   64455c1aaa47: Pull complete
   cdc57526dc05: Pull complete
   92b355b184ea: Pull complete
   5c5463f1346a: Pull complete
   Digest: sha256:42a5e73c830a2379e8f75b195cd53e43cb1c221ac1d75b91265ef0974de6482a
   Status: Downloaded newer image for golang:1.9-alpine
    ---> a4c7ad5b9041
   Step 2/10 : RUN apk add --no-cache     make     git
    ---> Running in 400795a9e773
   fetch http://dl-cdn.alpinelinux.org/alpine/v3.6/main/x86_64/APKINDEX.tar.gz
   fetch http://dl-cdn.alpinelinux.org/alpine/v3.6/community/x86_64/APKINDEX.tar.gz
   (1/6) Installing libssh2 (1.8.0-r1)
   (2/6) Installing libcurl (7.57.0-r0)
   (3/6) Installing expat (2.2.0-r1)
   (4/6) Installing pcre (8.41-r0)
   (5/6) Installing git (2.13.5-r0)
   (6/6) Installing make (4.2.1-r0)
   Executing busybox-1.26.2-r9.trigger
   OK: 25 MiB in 18 packages
   Removing intermediate container 400795a9e773
    ---> 02ad96bc3da0
   Step 3/10 : COPY . /go/src/github.com/pingcap/tidb
    ---> 08fa94416742
   Step 4/10 : WORKDIR /go/src/github.com/pingcap/tidb/
   Removing intermediate container a850a8ead739
    ---> d8b1af371946
   Step 5/10 : RUN make
    ---> Running in 4c92bc149091
   make parser
   make[1]: Entering directory '/go/src/github.com/pingcap/tidb'
   CGO_ENABLED=0 go build  -o bin/goyacc parser/goyacc/main.go
   bin/goyacc -o /dev/null parser/parser.y
   Parse table entries: 611181 of 1601340, x 16 bits == 1222362 bytes
   bin/goyacc -o parser/parser.go parser/parser.y 2>&1 | egrep "(shift|reduce)/reduce" | awk '{print} END {if (NR > 0) {print "Find conflict in parser.y. Please check y.output for more information."; exit 1;}}'
   rm -f y.output
   make[1]: Leaving directory '/go/src/github.com/pingcap/tidb'
   CGO_ENABLED=0 go build   -ldflags '-X "github.com/pingcap/tidb/mysql.TiDBReleaseVersion=v1.1.0-alpha.1-24-g96bd4e11-dirty" -X "github.com/pingcap/tidb/util/printer.TiDBBuildTS=2018-01-26 11:57:54" -X "github.com/pingcap/tidb/util/printer.TiDBGitHash=96bd4e11621665ec40934b35f1cee7b77aa8a851" -X "github.com/pingcap/tidb/util/printer.TiDBGitBranch=master" -X "github.com/pingcap/tidb/util/printer.GoVersion=go version go1.9.3 linux/amd64"' -o bin/tidb-server tidb-server/main.go
   Build TiDB Server successfully!
   Removing intermediate container 4c92bc149091
    ---> 16da7786d02c
   Step 6/10 : FROM scratch
    --->
   Step 7/10 : COPY --from=builder /go/src/github.com/pingcap/tidb/bin/tidb-server /tidb-server
    ---> 455347e6b0c2
   Step 8/10 : WORKDIR /
   Removing intermediate container 3f4572ccf0bc
    ---> 01bf74f5c71f
   Step 9/10 : EXPOSE 4000
    ---> Running in 23eaa5cc2adc
   Removing intermediate container 23eaa5cc2adc
    ---> fc91019cc0a5
   Step 10/10 : ENTRYPOINT ["/tidb-server"]
    ---> Running in 0773ce0ba0a1
   Removing intermediate container 0773ce0ba0a1
    ---> aaa9c01eaaf9
   Successfully built aaa9c01eaaf9
```
@sre-bot
Copy link
Contributor

sre-bot commented Jan 26, 2018

Hi contributor, thanks for your PR.

This patch needs to be approved by someone of admins. They should reply with "/ok-to-test" to accept this PR for running test automatically.

@ngaut
Copy link
Member

ngaut commented Jan 26, 2018

Nice clean up, thank you @im-kulikov

@im-kulikov
Copy link
Contributor Author

im-kulikov commented Jan 26, 2018

dep 0.4.1 provide prune option to Gopkg.toml


@ngaut you're welcome 😉

@shenli
Copy link
Member

shenli commented Jan 26, 2018

@im-kulikov Thanks!

@shenli
Copy link
Member

shenli commented Jan 26, 2018

/ok-to-test

@shenli
Copy link
Member

shenli commented Jan 26, 2018

/run-all-tests

@shenli
Copy link
Member

shenli commented Jan 26, 2018

@zhexuany PTAL

@zhexuany
Copy link
Contributor

zhexuany commented Jan 26, 2018

@im-kulikov

Thanks PR. I planed to do this sever days ago but my hand was tied. Thanks for this!!!

Although this is a great PR, something I want to point out. Files like AUTHORS and CONTRIBUTORS should be also excluded. Please also update hack/clean_vendor.sh. Additionally,
you probably need update Makefile too. as you pointed out dep ~~~absurd~~~ absorbed prune into ensure

@im-kulikov
Copy link
Contributor Author

im-kulikov commented Jan 26, 2018

@zhexuany dep prune not needed now, it always run with dep ensure or dep ensure -update

AUTHORS, CONTRIBUTORS, LICENSE not removed by dep prune (see golang/dep#944)

there are not need to add dep prune to hack/clean_vendor.sh, because, as I say above it already integrated to ensure:

You can see message like this if run dep prune:

Pruning is now performed automatically by dep ensure.
Set prune settings in Gopkg.toml and it it will be applied when running ensure.

This command currently still prunes as it always has, to ease the transition.
However, it will be removed in a future version of dep.

Now is the time to update your Gopkg.toml and remove `dep prune` from any scripts.

I can manually remove files (AUTHORS, CONTRIBUTORS, LICENSE) if you wanna (for what 🙂)

- cleanup empty folders (and _vendor)
- add cleanup commands to `hack/cleanup_vendor.sh`
@im-kulikov
Copy link
Contributor Author

@zhexuany fixed by d660234

@zhexuany
Copy link
Contributor

zhexuany commented Jan 26, 2018

Yeah, I made a spelling mistake and sorry for the confusion. By saying "you probably need update Makefile", I mean you need remove dep prune from Makefile.

By saying "Please also update hack/clean_vendor.sh", I mean you need remove dep prune from it. I just checked current hack/clean_vendor.sh version. There is no dep prunepresent. So just discard this one.

Before dep 0.4.1, our workflow is first run dep ensure and then dep prune and then bash hack/clean_vendor.sh. The script ~~~also~~~ already takes care of files like AUTHORS. You may just run this script.

I am so sorry for the confusion. I hope this ~~~clarify~~~ clarifies my last reply. Thanks for the contribution. 👍

@im-kulikov
Copy link
Contributor Author

im-kulikov commented Jan 26, 2018

Oh.. understood 😅

@zhexuany Can you check current changes?

I’ll add small changes to ‘hack/clean_vendor.sh’ and remove unused files / empty folders and _vendor folders

@@ -1,4 +1,8 @@
dep ensure &>/dev/null
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not need this.
dep ensure will be ran outside this script.

find vendor \( -type f -or -type l \) -not -name "*.go" -not -name "LICENSE" -not -name "*.s" -not -name "PATENTS" -not -name "*.h" -not -name "*.c" | xargs -I {} rm {}
# delete all test files
find vendor -type f -name "*_generated.go" | xargs -I {} rm {}
find vendor -type f -name "*_test.go" | xargs -I {} rm {}
find vendor -type f -name "AUTHORS" -name "AUTHORS.md" -iname 'CONTRIBUTORS' -iname 'NOTICE' | xargs -I {} rm {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is duplicated.
find vendor \( -type f -or -type l \) -not -name "*.go" -not -name "LICENSE" -not -name "*.s" -not -name "PATENTS" -not -name "*.h" -not -name "*.c" | xargs -I {} rm {} will delete all file expect ".go", ".s", "PATENTS" ,and "LICENSE"".

@im-kulikov
Copy link
Contributor Author

@zhexuany fixed, thanks

@zhexuany
Copy link
Contributor

LGTM @shenli PTAL

@zhexuany zhexuany added the status/LGT1 Indicates that a PR has LGTM 1. label Jan 28, 2018
@ngaut
Copy link
Member

ngaut commented Jan 29, 2018

/run-all-tests

@im-kulikov
Copy link
Contributor Author

@ngaut / @zhexuany good news, all tests have passed :)

Copy link
Member

@ngaut ngaut left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ngaut ngaut merged commit cfe44e8 into pingcap:master Jan 29, 2018
@ngaut
Copy link
Member

ngaut commented Jan 29, 2018

Thank you for your contribution. @im-kulikov

@im-kulikov im-kulikov deleted the cleanup-vendor branch January 29, 2018 13:01
@zhexuany
Copy link
Contributor

@im-kulikov Thanks for you contribution. Hope can review you next PR. 💯

@im-kulikov
Copy link
Contributor Author

@zhexuany oh, thanks)

@sre-bot sre-bot added the contribution This PR is from a community contributor. label Dec 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution This PR is from a community contributor. status/LGT1 Indicates that a PR has LGTM 1.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants