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.

Sign up for GitHub

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

Jump to bottom

cmd/go: skip issue33139 when the 'cc' script command is unavailable #43912

Closed
wants to merge 1 commit into from
Closed

cmd/go: skip issue33139 when the 'cc' script command is unavailable #43912

wants to merge 1 commit into from

Conversation

nehaljwani
Copy link
Contributor

@nehaljwani nehaljwani commented Jan 26, 2021 •
edited
Loading

With CGO disabled, the test suite tries to run the following and fail:

CGO_ENABLED=0 go test -run=TestScript/link_syso_issue33139 cmd/go
go test proxy running at GOPROXY=http://127.0.0.1:38829/mod
--- FAIL: TestScript (0.01s)
--- FAIL: TestScript/link_syso_issue33139 (0.01s)
script_test.go:215:
# Test that we can use the external linker with a host syso file that is
# embedded in a package, that is referenced by a Go assembly function.
# See issue 33139. (0.000s)
# External linking is not supported on linux/ppc64.
# See: #8912 (0.000s)
# External linking is not supported on linux/riscv64.
# See: #36739 (0.001s)
> [linux] [riscv64] skip
> cc -c -o syso/objTestImpl.syso syso/src/objTestImpl.c
FAIL: testdata/script/link_syso_issue33139.txt:15:
unexpected error starting command:
fork/exec /dev/null: permission denied

CC was set to /dev/null (during build) in the scenario mentioned above

This patch replaces [!exec:cc] with [!cgo] because we care about the
availability of the 'cc' builtin and not the 'cc' executable in $PATH

@google-cla google-cla bot added the cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change. label Jan 26, 2021
@gopherbot
Copy link
Contributor

This PR (HEAD: d0e7925) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/286633 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Ian Lance Taylor:

Patch Set 1: Run-TryBot+1

Please don’t reply on this GitHub thread. Visit golang.org/cl/286633.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 1:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=6e15e7f6

Please don’t reply on this GitHub thread. Visit golang.org/cl/286633.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 1: TryBot-Result+1

TryBots are happy.

Please don’t reply on this GitHub thread. Visit golang.org/cl/286633.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Bryan C. Mills:

Patch Set 1:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/286633.
After addressing review feedback, remember to publish your drafts!

@nehaljwani nehaljwani force-pushed the disable-issue33139-nocgo branch from d0e7925 to 51df6bd Compare January 26, 2021 16:11
@gopherbot
Copy link
Contributor

This PR (HEAD: 51df6bd) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/286633 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@nehaljwani nehaljwani force-pushed the disable-issue33139-nocgo branch from 51df6bd to 502c3d6 Compare January 26, 2021 16:16
@gopherbot
Copy link
Contributor

This PR (HEAD: 502c3d6) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/286633 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@nehaljwani nehaljwani force-pushed the disable-issue33139-nocgo branch from 502c3d6 to d0af87e Compare January 26, 2021 16:18
@gopherbot
Copy link
Contributor

This PR (HEAD: d0af87e) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/286633 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Nehal J Wani:

Patch Set 4:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/286633.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Cherry Zhang:

Patch Set 4:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/286633.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Bryan C. Mills:

Patch Set 4:

(3 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/286633.
After addressing review feedback, remember to publish your drafts!

@nehaljwani nehaljwani force-pushed the disable-issue33139-nocgo branch from d0af87e to 4bc23bb Compare January 26, 2021 21:51
@gopherbot
Copy link
Contributor

This PR (HEAD: 4bc23bb) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/286633 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@nehaljwani nehaljwani changed the title cmd/go: issue33139 is only valid when running with CGO cmd/go: issue33139 is valid only when 'cc' builtin is available Jan 26, 2021
@nehaljwani nehaljwani force-pushed the disable-issue33139-nocgo branch from 4bc23bb to 88f8e38 Compare January 26, 2021 21:53
@gopherbot
Copy link
Contributor

This PR (HEAD: 88f8e38) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/286633 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@nehaljwani nehaljwani force-pushed the disable-issue33139-nocgo branch from 88f8e38 to e24c5f6 Compare January 26, 2021 21:56
@gopherbot
Copy link
Contributor

This PR (HEAD: e24c5f6) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/286633 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Nehal J Wani:

Patch Set 7:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/286633.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Bryan C. Mills:

Patch Set 7: Run-TryBot+1 Code-Review+2

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/286633.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 7:

SlowBots beginning. Status page: https://farmer.golang.org/try?commit=c1a85ab9

Please don’t reply on this GitHub thread. Visit golang.org/cl/286633.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 7:

Build is still in progress...
This change failed on linux-amd64-clang:
See https://storage.googleapis.com/go-build-log/c1a85ab9/linux-amd64-clang_fe871a86.log

Other builds still in progress; subsequent failure notices suppressed until final report. Consult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test exactly your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.

Please don’t reply on this GitHub thread. Visit golang.org/cl/286633.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 7: TryBot-Result-1

1 of 24 SlowBots failed:
Failed on linux-amd64-clang: https://storage.googleapis.com/go-build-log/c1a85ab9/linux-amd64-clang_fe871a86.log

Consult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test exactly your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.

SlowBot builds that ran:

  • linux-amd64-clang
  • linux-amd64-longtest
  • linux-amd64-nocgo

Please don’t reply on this GitHub thread. Visit golang.org/cl/286633.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Bryan C. Mills:

Patch Set 7:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/286633.
After addressing review feedback, remember to publish your drafts!

@nehaljwani
cmd/go: issue33139 is valid only when 'cc' builtin is available
3b74378 
With CGO disabled, the test suite tries to run the following and fail:

CGO_ENABLED=0 go test -run=TestScript/link_syso_issue33139 cmd/go
go test proxy running at GOPROXY=http://127.0.0.1:38829/mod
--- FAIL: TestScript (0.01s)
    --- FAIL: TestScript/link_syso_issue33139 (0.01s)
        script_test.go:215:
            # Test that we can use the external linker with a host syso file that is
            # embedded in a package, that is referenced by a Go assembly function.
            # See issue 33139. (0.000s)
            # External linking is not supported on linux/ppc64.
            # See: golang#8912 (0.000s)
            # External linking is not supported on linux/riscv64.
            # See: golang#36739 (0.001s)
            > [linux] [riscv64] skip
            > cc -c -o syso/objTestImpl.syso syso/src/objTestImpl.c
            FAIL: testdata/script/link_syso_issue33139.txt:15:
                unexpected error starting command:
                        fork/exec /dev/null: permission denied

CC was set to /dev/null (during build) in the scenario mentioned above

This patch replaces [!exec:cc] with [!cgo] because we care about the
availability of the 'cc' builtin and not the 'cc' executable in $PATH
@nehaljwani nehaljwani force-pushed the disable-issue33139-nocgo branch from e24c5f6 to 3b74378 Compare January 27, 2021 15:53
@gopherbot
Copy link
Contributor

This PR (HEAD: 3b74378) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/286633 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Nehal J Wani:

Patch Set 8:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/286633.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Bryan C. Mills:

Patch Set 8: Run-TryBot+1

Please don’t reply on this GitHub thread. Visit golang.org/cl/286633.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 8:

SlowBots beginning. Status page: https://farmer.golang.org/try?commit=b6f1bca8

Please don’t reply on this GitHub thread. Visit golang.org/cl/286633.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 8: TryBot-Result+1

SlowBots are happy.

SlowBot builds that ran:

  • linux-amd64-clang
  • linux-amd64-longtest
  • linux-amd64-nocgo

Please don’t reply on this GitHub thread. Visit golang.org/cl/286633.
After addressing review feedback, remember to publish your drafts!

@bcmills bcmills changed the title cmd/go: issue33139 is valid only when 'cc' builtin is available cmd/go: skip issue33139 when the 'cc' script command is unavailable Jan 27, 2021
@gopherbot
Copy link
Contributor

Message from Jay Conrod:

Patch Set 9: Trust+1

Please don’t reply on this GitHub thread. Visit golang.org/cl/286633.
After addressing review feedback, remember to publish your drafts!

gopherbot pushed a commit that referenced this pull request Jan 27, 2021
@nehaljwani
cmd/go: skip issue33139 when the 'cc' script command is unavailable
ff9e836 
With CGO disabled, the test suite tries to run the following and fail:

CGO_ENABLED=0 go test -run=TestScript/link_syso_issue33139 cmd/go
go test proxy running at GOPROXY=http://127.0.0.1:38829/mod
--- FAIL: TestScript (0.01s)
    --- FAIL: TestScript/link_syso_issue33139 (0.01s)
        script_test.go:215:
            # Test that we can use the external linker with a host syso file that is
            # embedded in a package, that is referenced by a Go assembly function.
            # See issue 33139. (0.000s)
            # External linking is not supported on linux/ppc64.
            # See: #8912 (0.000s)
            # External linking is not supported on linux/riscv64.
            # See: #36739 (0.001s)
            > [linux] [riscv64] skip
            > cc -c -o syso/objTestImpl.syso syso/src/objTestImpl.c
            FAIL: testdata/script/link_syso_issue33139.txt:15:
                unexpected error starting command:
                        fork/exec /dev/null: permission denied

CC was set to /dev/null (during build) in the scenario mentioned above

This patch replaces [!exec:cc] with [!cgo] because we care about the
availability of the 'cc' builtin and not the 'cc' executable in $PATH

Change-Id: Ifbd2441f5f8e903ca3da213aba76f44c2e2eebab
GitHub-Last-Rev: 3b74378
GitHub-Pull-Request: #43912
Reviewed-on: https://go-review.googlesource.com/c/go/+/286633
Trust: Jay Conrod <jayconrod@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
@gopherbot
Copy link
Contributor

Message from Bryan C. Mills:

Patch Set 9:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/286633.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

This PR is being closed because golang.org/cl/286633 has been merged.

@gopherbot gopherbot closed this Jan 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@nehaljwani @gopherbot