From 352a4147874a496be4d33861b974eb30eaa5c630 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Wed, 20 Dec 2023 08:02:55 +0100 Subject: [PATCH] Address review comments --- go/private/actions/compilepkg.bzl | 4 +-- go/tools/builders/BUILD.bazel | 1 - go/tools/builders/cgo2.go | 33 +++++++++++++++++++ go/tools/builders/compilepkg.go | 32 +++++++++---------- go/tools/builders/utils.go | 53 ------------------------------- 5 files changed, 50 insertions(+), 73 deletions(-) delete mode 100644 go/tools/builders/utils.go diff --git a/go/private/actions/compilepkg.bzl b/go/private/actions/compilepkg.bzl index 2f1429a89c..10fa6970a2 100644 --- a/go/private/actions/compilepkg.bzl +++ b/go/private/actions/compilepkg.bzl @@ -124,8 +124,8 @@ def emit_compilepkg( args.add("-p", importmap) args.add("-package_list", go.package_list) - args.add("-o", out_lib) - args.add("-x", out_export) + args.add("-lo", out_lib) + args.add("-o", out_export) if nogo: args.add_all(archives, before_each = "-facts", map_each = _facts) inputs.extend([archive.data.facts_file for archive in archives if archive.data.facts_file]) diff --git a/go/tools/builders/BUILD.bazel b/go/tools/builders/BUILD.bazel index 1cc954b204..d327a3afbb 100644 --- a/go/tools/builders/BUILD.bazel +++ b/go/tools/builders/BUILD.bazel @@ -80,7 +80,6 @@ filegroup( "replicate.go", "stdlib.go", "stdliblist.go", - "utils.go", ] + select({ "@bazel_tools//src/conditions:windows": ["path_windows.go"], "//conditions:default": ["path.go"], diff --git a/go/tools/builders/cgo2.go b/go/tools/builders/cgo2.go index fc2876a994..80043e467b 100644 --- a/go/tools/builders/cgo2.go +++ b/go/tools/builders/cgo2.go @@ -23,9 +23,11 @@ package main import ( "bytes" "fmt" + "io" "io/ioutil" "os" "path/filepath" + "runtime" "strings" ) @@ -395,3 +397,34 @@ func (e cgoError) Error() string { fmt.Fprintf(b, "Ensure that 'cgo = True' is set and the C/C++ toolchain is configured.") return b.String() } + +func copyFile(inPath, outPath string) error { + inFile, err := os.Open(inPath) + if err != nil { + return err + } + defer inFile.Close() + outFile, err := os.OpenFile(outPath, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0666) + if err != nil { + return err + } + defer outFile.Close() + _, err = io.Copy(outFile, inFile) + return err +} + +func linkFile(inPath, outPath string) error { + inPath, err := filepath.Abs(inPath) + if err != nil { + return err + } + return os.Symlink(inPath, outPath) +} + +func copyOrLinkFile(inPath, outPath string) error { + if runtime.GOOS == "windows" { + return copyFile(inPath, outPath) + } else { + return linkFile(inPath, outPath) + } +} diff --git a/go/tools/builders/compilepkg.go b/go/tools/builders/compilepkg.go index 70cbc14ed1..360f69fe0d 100644 --- a/go/tools/builders/compilepkg.go +++ b/go/tools/builders/compilepkg.go @@ -52,7 +52,7 @@ func compilePkg(args []string) error { var unfilteredSrcs, coverSrcs, embedSrcs, embedLookupDirs, embedRoots, recompileInternalDeps multiFlag var deps, facts archiveMultiFlag var importPath, packagePath, nogoPath, packageListPath, coverMode string - var outPath, outXPath, outFactsPath, cgoExportHPath string + var outLinkobjPath, outInterfacePath, outFactsPath, cgoExportHPath string var testFilter string var gcFlags, asmFlags, cppFlags, cFlags, cxxFlags, objcFlags, objcxxFlags, ldFlags quoteMultiFlag var coverFormat string @@ -77,8 +77,8 @@ func compilePkg(args []string) error { fs.StringVar(&nogoPath, "nogo", "", "The nogo binary. If unset, nogo will not be run.") fs.StringVar(&packageListPath, "package_list", "", "The file containing the list of standard library packages") fs.StringVar(&coverMode, "cover_mode", "", "The coverage mode to use. Empty if coverage instrumentation should not be added.") - fs.StringVar(&outPath, "o", "", "The full output archive file required by the linker") - fs.StringVar(&outXPath, "x", "", "The export-only output archive required to compile dependent packages") + fs.StringVar(&outLinkobjPath, "lo", "", "The full output archive file required by the linker") + fs.StringVar(&outInterfacePath, "o", "", "The export-only output archive required to compile dependent packages") fs.StringVar(&outFactsPath, "out_facts", "", "The file to emit serialized nogo facts to (must be set if -nogo is set") fs.StringVar(&cgoExportHPath, "cgoexport", "", "The _cgo_exports.h file to write") fs.StringVar(&testFilter, "testfilter", "off", "Controls test package filtering") @@ -96,7 +96,7 @@ func compilePkg(args []string) error { } cgoEnabled := os.Getenv("CGO_ENABLED") == "1" cc := os.Getenv("CC") - outPath = abs(outPath) + outLinkobjPath = abs(outLinkobjPath) for i := range unfilteredSrcs { unfilteredSrcs[i] = abs(unfilteredSrcs[i]) } @@ -162,8 +162,8 @@ func compilePkg(args []string) error { ldFlags, nogoPath, packageListPath, - outPath, - outXPath, + outLinkobjPath, + outInterfacePath, outFactsPath, cgoExportHPath, coverFormat, @@ -195,8 +195,8 @@ func compileArchive( ldFlags []string, nogoPath string, packageListPath string, - outPath string, - outXPath string, + outLinkObj string, + outInterfacePath string, outFactsPath string, cgoExportHPath string, coverFormat string, @@ -215,7 +215,7 @@ func compileArchive( // Otherwise, GoPack will complain if we try to add assembly or cgo objects. // A truly empty archive does not include any references to source file paths, which // ensures hermeticity even though the temp file path is random. - emptyGoFile, err := os.CreateTemp(filepath.Dir(outPath), "*.go") + emptyGoFile, err := os.CreateTemp(filepath.Dir(outLinkObj), "*.go") if err != nil { return err } @@ -406,7 +406,7 @@ func compileArchive( } // Build an importcfg file for the compiler. - importcfgPath, err := buildImportcfgFileForCompile(imports, goenv.installSuffix, filepath.Dir(outPath)) + importcfgPath, err := buildImportcfgFileForCompile(imports, goenv.installSuffix, filepath.Dir(outLinkObj)) if err != nil { return err } @@ -483,7 +483,7 @@ func compileArchive( } // Compile the filtered .go files. - if err := compileGo(goenv, goSrcs, packagePath, importcfgPath, embedcfgPath, asmHdrPath, symabisPath, gcFlags, pgoprofile, outPath, outXPath); err != nil { + if err := compileGo(goenv, goSrcs, packagePath, importcfgPath, embedcfgPath, asmHdrPath, symabisPath, gcFlags, pgoprofile, outLinkObj, outInterfacePath); err != nil { return err } @@ -517,7 +517,7 @@ func compileArchive( // Pack .o files into the archive. These may come from cgo generated code, // cgo dependencies (cdeps), or assembly. if len(objFiles) > 0 { - if err := appendToArchive(goenv, outPath, objFiles); err != nil { + if err := appendToArchive(goenv, outLinkObj, objFiles); err != nil { return err } } @@ -535,7 +535,7 @@ func compileArchive( return nil } -func compileGo(goenv *env, srcs []string, packagePath, importcfgPath, embedcfgPath, asmHdrPath, symabisPath string, gcFlags []string, pgoprofile, outPath, outXPath string) error { +func compileGo(goenv *env, srcs []string, packagePath, importcfgPath, embedcfgPath, asmHdrPath, symabisPath string, gcFlags []string, pgoprofile, outLinkobjPath, outInterfacePath string) error { args := goenv.goTool("compile") args = append(args, "-p", packagePath, "-importcfg", importcfgPath, "-pack") if embedcfgPath != "" { @@ -551,8 +551,8 @@ func compileGo(goenv *env, srcs []string, packagePath, importcfgPath, embedcfgPa args = append(args, "-pgoprofile", pgoprofile) } args = append(args, gcFlags...) - args = append(args, "-o", outXPath) - args = append(args, "-linkobj", outPath) + args = append(args, "-o", outInterfacePath) + args = append(args, "-linkobj", outLinkobjPath) args = append(args, "--") args = append(args, srcs...) absArgs(args, []string{"-I", "-o", "-trimpath", "-importcfg"}) @@ -600,8 +600,6 @@ func runNogo(ctx context.Context, workDir string, nogoPath string, srcs []string func appendToArchive(goenv *env, outPath string, objFiles []string) error { // Use abs to work around long path issues on Windows. - // TODO(jayconrod): copy cmd/internal/archive and use that instead of - // shelling out to cmd/pack. args := goenv.goTool("pack", "r", abs(outPath)) args = append(args, objFiles...) return goenv.runCommand(args) diff --git a/go/tools/builders/utils.go b/go/tools/builders/utils.go deleted file mode 100644 index 0ac0d89b58..0000000000 --- a/go/tools/builders/utils.go +++ /dev/null @@ -1,53 +0,0 @@ -// Copyright 2017 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package main - -import ( - "io" - "os" - "path/filepath" - "runtime" -) - -func copyFile(inPath, outPath string) error { - inFile, err := os.Open(inPath) - if err != nil { - return err - } - defer inFile.Close() - outFile, err := os.OpenFile(outPath, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0666) - if err != nil { - return err - } - defer outFile.Close() - _, err = io.Copy(outFile, inFile) - return err -} - -func linkFile(inPath, outPath string) error { - inPath, err := filepath.Abs(inPath) - if err != nil { - return err - } - return os.Symlink(inPath, outPath) -} - -func copyOrLinkFile(inPath, outPath string) error { - if runtime.GOOS == "windows" { - return copyFile(inPath, outPath) - } else { - return linkFile(inPath, outPath) - } -}