Skip to content

go {build,test}: rm -i option - #2751

Merged
yongtang merged 1 commit into
moby:masterfrom
kolyshkin:no-i
Sep 25, 2018
Merged

go {build,test}: rm -i option#2751
yongtang merged 1 commit into
moby:masterfrom
kolyshkin:no-i

Conversation

@kolyshkin

@kolyshkin kolyshkin commented Sep 18, 2018

Copy link
Copy Markdown
Contributor

Looks like it was used to get faster incremental builds.
Nowdays (since Go 1.10) there is no need to use it, because
go build cache is used [1].

This fixes make binaries on my system where golang is installed
as read-only snap:

$ make binaries
🐳 bin/swarmd
go build runtime/cgo: open /snap/go/2635/pkg/linux_amd64/runtime/cgo.a: read-only file system
direct.mk:100: recipe for target 'bin/swarmd' failed

[1] https://groups.google.com/forum/#!msg/golang-dev/qfa3mHN4ZPA/X2UzjNV1BAAJ

Looks like it was used to get faster incremental builds.
Nowdays (since Go 1.10) there is no need to use it, because
go build cache is used [1].

This fixes `make binaries` on my system where golang is installed
as read-only snap:

> $ make binaries
> 🐳 bin/swarmd
> go build runtime/cgo: open /snap/go/2635/pkg/linux_amd64/runtime/cgo.a: read-only file system
> direct.mk:100: recipe for target 'bin/swarmd' failed

[1] https://groups.google.com/forum/#!msg/golang-dev/qfa3mHN4ZPA/X2UzjNV1BAAJ

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@codecov

codecov Bot commented Sep 18, 2018

Copy link
Copy Markdown

Codecov Report

Merging #2751 into master will decrease coverage by 0.02%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2751      +/-   ##
==========================================
- Coverage   61.77%   61.74%   -0.03%     
==========================================
  Files         134      134              
  Lines       21890    21890              
==========================================
- Hits        13522    13517       -5     
  Misses       6911     6911              
- Partials     1457     1462       +5

@kolyshkin

Copy link
Copy Markdown
Contributor Author

@anshulpundir @nishanttotla @thaJeztah PTAL

Similar PR: moby/moby#37879

@yongtang yongtang left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@thaJeztah thaJeztah left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

don't have permissions to merge though 😄

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

Successfully merging this pull request may close these issues.

3 participants