[gndoc] Removing links to 3p build files
This removes the link generation from gndoc. The command line option for
the jiri project sources is still present, but is not longer used. This
is to allow for checking in the gndoc updates, but not break the recipe
used to run gndoc and check in the generated documentation.
A followup CL will remove the option after the recipe has been update to
not pass the argument to gndoc.
Bug: 119650
Change-Id: Ia7f6fd526190768fa616879d68c818a08a516931
Reviewed-on: https://fuchsia-review.googlesource.com/c/tools/+/789648
Commit-Queue: Clayton Wilkinson <wilkinsonclay@google.com>
Reviewed-by: Catherine Duncan <catduncan@google.com>
diff --git a/gndoc/argmap.go b/gndoc/argmap.go
index 24888a6..eae9cf5 100644
--- a/gndoc/argmap.go
+++ b/gndoc/argmap.go
@@ -14,15 +14,13 @@
type ArgMap struct {
argLookup map[string][]string // map of arg names to keys for lookup
args map[string][]Arg // map of key names to relevant args
- sources *SourceMap // map of existing source locations for linkifying
}
// NewArgMap returns a pointer to an
-func NewArgMap(sources *SourceMap) *ArgMap {
+func NewArgMap() *ArgMap {
return &ArgMap{
argLookup: make(map[string][]string),
args: make(map[string][]Arg),
- sources: sources,
}
}
@@ -94,7 +92,7 @@
}
for _, argsKey := range sortedArgs.m[name].k {
gnArgs := sortedArgs.m[name].m[argsKey]
- writeArgs(gnArgs, out, a.sources)
+ writeArgs(gnArgs, out)
}
}
}
diff --git a/gndoc/argmap_test.go b/gndoc/argmap_test.go
index 005c97b..7c16ada 100644
--- a/gndoc/argmap_test.go
+++ b/gndoc/argmap_test.go
@@ -12,7 +12,7 @@
)
func genArgMapInRandomOrder() *ArgMap {
- testArgMap := NewArgMap(Sources())
+ testArgMap := NewArgMap()
gnArgs := []Arg{
defaultx64Arg,
defaultarm64Arg,
@@ -44,6 +44,7 @@
## ` + "`target_cpu = arm64, package='other/package/default'`" + `
### arm64Other
+
Description of arm64 arg.
**Current value for ` + "`target_cpu = arm64, package='other/package/default'`:** `arg`" + `
@@ -53,6 +54,7 @@
## ` + "`target_cpu = arm64, target_cpu = arm64, package='test/package/default'`" + `
### arm64
+
Description of arm64 arg.
**Current value for ` + "`target_cpu = arm64`:** `arg`" + `
@@ -66,6 +68,7 @@
## ` + "`target_cpu = arm64, target_cpu = x64`" + `
### default
+
Description of default arg.
**Current value (from the default):** ` + "`false`" + `
@@ -73,37 +76,40 @@
From //test/BUILD.gn:2
### default_current
+
Description of default_current arg.
**Current value for ` + "`target_cpu = arm64`:** `[1, 2]`" + `
-From [//build/BUILD.gn:24](http://fuchsia.com/build/BUILD.gn#24)
+From //build/BUILD.gn:24
**Overridden from the default:** ` + "`[3, 4]`" + `
-From [//base/BUILD.gn:4](http://fuchsia.com/base/BUILD.gn#4)
+From //base/BUILD.gn:4
**Current value for ` + "`target_cpu = x64`:** `3`" + `
-From [//build/BUILD.gn:24](http://fuchsia.com/build/BUILD.gn#24)
+From //build/BUILD.gn:24
**Overridden from the default:** ` + "`4`" + `
-From [//base/BUILD.gn:2](http://fuchsia.com/base/BUILD.gn#2)
+From //base/BUILD.gn:2
## ` + "`target_cpu = x64, package='other/package/default'`" + `
### NewLine
+
Description of newline arg.
**Current value (from the default):**
-` + "```" + `
+` + "\n```none" + `
{
base = "//build/toolchain/fuchsia:x64"
}
` + "```" + `
### x64Other
+
Description of x64 arg.
**Current value for ` + "`target_cpu = x64, package='other/package/default'`:** `arg`" + `
@@ -113,7 +119,8 @@
## ` + "`target_cpu = x64, target_cpu = x64, package='test/package/default'`" + `
### x64
-Description of x64 arg that references [//build/path.py](http://fuchsia.com/build/path.py), //sources, and [//base](http://fuchsia.com/base).
+
+Description of x64 arg that references //build/path.py, //sources, and //base.
**Current value for ` + "`target_cpu = x64`:** `1`" + `
diff --git a/gndoc/cmd/main.go b/gndoc/cmd/main.go
index 37add2e..cc750cb 100644
--- a/gndoc/cmd/main.go
+++ b/gndoc/cmd/main.go
@@ -47,23 +47,15 @@
flag.PrintDefaults()
}
- if sourcesFile == "" {
- log.Fatalf("Error: no Jiri project reference file specified.\nTo generate, use jiri project -json-output=<file>.\n")
- }
-
- // Open the json file.
- sources, err := os.Open(sourcesFile)
- if err != nil {
- log.Fatalf("Error: unable to open %s", sourcesFile)
- }
- defer sources.Close()
-
+ // sources is no longer used since the URL generated for remotes is can be wrong
+ // due to the path within the Fuchsia tree not matching the path from the root
+ // of the remote project.
+ // Also, it appears some BUILD.gn files in third_party are being generated from Bazel
+ // builds, so the link is not accurate.
+ //
+ // TODO(fxb/119650): Remove sourcemap all together.
ctx := context.Background()
- sourceMap, err := gndoc.NewSourceMap(sources)
- if err != nil {
- log.Fatalf("Error: %s", err)
- }
- argMap := gndoc.NewArgMap(sourceMap)
+ argMap := gndoc.NewArgMap()
args, errs := gndoc.ParseGNArgs(ctx, inputFiles, keyArgs)
argMap.AddArgs(args)
diff --git a/gndoc/formatter.go b/gndoc/formatter.go
index 15119f0..3e92edc 100644
--- a/gndoc/formatter.go
+++ b/gndoc/formatter.go
@@ -22,7 +22,7 @@
)
// writeArg emits the name, comment description, and value(s) of the argument in Markdown.
-func writeArgs(args []Arg, out io.Writer, sources *SourceMap) {
+func writeArgs(args []Arg, out io.Writer) {
if len(args) == 0 {
return
}
@@ -30,17 +30,18 @@
return args[i].Name < args[j].Name
})
- fmt.Fprintf(out, "%s %s\n", strings.Repeat("#", nameDepth), args[0].Name)
+ // Include a blank line after heading for properly formatted markdown.
+ fmt.Fprintf(out, "%s %s\n\n", strings.Repeat("#", nameDepth), args[0].Name)
// TODO (juliehockett): Make sure that *all* comments get emitted.
- writeLinkifiedComment(&args[0], out, sources)
- writeAllValues(args, out, sources)
+ writeLinkifiedComment(&args[0], out)
+ writeAllValues(args, out)
}
// writeValue emits the value of a given argument value, along with the associated Markdown link to its declaration and build (if present).
-func writeValue(a *argValue, out io.Writer, sources *SourceMap) {
+func writeValue(a *argValue, out io.Writer) {
var value string
if strings.Contains(a.Val, "\n") {
- value = fmt.Sprintf("\n```\n%s\n```", a.Val)
+ value = fmt.Sprintf("\n\n```none\n%s\n```", a.Val)
} else {
value = fmt.Sprintf(" `%s`", a.Val)
}
@@ -49,37 +50,27 @@
// If there is no declaration file, emit just the value.
fmt.Fprintf(out, "%s\n\n", value)
} else {
- // Otherwise, emit the value with a link to the declaration.
- link := sources.GetSourceLink(a.File, a.Line)
- if link == "" {
- fmt.Fprintf(out, "%s\n\nFrom %s:%d\n\n", value, a.File, a.Line)
- return
- }
- fmt.Fprintf(out, "%s\n\nFrom [%s:%d](%s)\n\n", value, a.File, a.Line, link)
+ fmt.Fprintf(out, "%s\n\nFrom %s:%d\n\n", value, a.File, a.Line)
}
}
-func writeLinkifiedComment(a *Arg, out io.Writer, sources *SourceMap) {
- replFunc := func(str string) string {
- if link := sources.GetSourceLink(str, 0); link != "" {
- return fmt.Sprintf("[%s](%s)", str, link)
- }
- return str
+func writeLinkifiedComment(a *Arg, out io.Writer) {
+ if a.Comment != "" {
+ fmt.Fprintf(out, "%s\n", a.Comment)
}
- fmt.Fprintf(out, "%s\n", linkRegexp.ReplaceAllStringFunc(a.Comment, replFunc))
}
-func writeAllValues(args []Arg, out io.Writer, sources *SourceMap) {
+func writeAllValues(args []Arg, out io.Writer) {
emptyArgValue := argValue{}
for _, a := range args {
if a.CurrentVal == emptyArgValue || a.CurrentVal == a.DefaultVal {
fmt.Fprintf(out, "**Current value (from the default):**")
- writeValue(&a.DefaultVal, out, sources)
+ writeValue(&a.DefaultVal, out)
return
}
fmt.Fprintf(out, "**Current value for `%s`:**", a.Key)
- writeValue(&a.CurrentVal, out, sources)
+ writeValue(&a.CurrentVal, out)
fmt.Fprintf(out, "**Overridden from the default:**")
- writeValue(&a.DefaultVal, out, sources)
+ writeValue(&a.DefaultVal, out)
}
}
diff --git a/gndoc/formatter_test.go b/gndoc/formatter_test.go
index a72757c..999026f 100644
--- a/gndoc/formatter_test.go
+++ b/gndoc/formatter_test.go
@@ -150,17 +150,10 @@
}
)
-func Sources() *SourceMap {
- s := SourceMap(make(map[string]string))
- s["base"] = "http://fuchsia.com/base"
- s["build"] = "http://fuchsia.com/build"
- return &s
-}
-
func TestDefault(t *testing.T) {
gnArgs := []Arg{defaultx64Arg, defaultarm64Arg}
- argMap := NewArgMap(Sources())
+ argMap := NewArgMap()
for _, arg := range gnArgs {
argMap.AddArg(arg)
}
@@ -175,6 +168,7 @@
## All builds
### default
+
Description of default arg.
**Current value (from the default):** ` + "`false`" + `
@@ -190,7 +184,7 @@
func TestDefaultWithCurrent(t *testing.T) {
gnArgs := []Arg{defaultx64ArgWithCurrent, defaultarm64ArgWithCurrent}
- argMap := NewArgMap(Sources())
+ argMap := NewArgMap()
for _, arg := range gnArgs {
argMap.AddArg(arg)
}
@@ -205,23 +199,24 @@
## All builds
### default_current
+
Description of default_current arg.
**Current value for ` + "`target_cpu = arm64`:** `[1, 2]`" + `
-From [//build/BUILD.gn:24](http://fuchsia.com/build/BUILD.gn#24)
+From //build/BUILD.gn:24
**Overridden from the default:** ` + "`[3, 4]`" + `
-From [//base/BUILD.gn:4](http://fuchsia.com/base/BUILD.gn#4)
+From //base/BUILD.gn:4
**Current value for ` + "`target_cpu = x64`:** `3`" + `
-From [//build/BUILD.gn:24](http://fuchsia.com/build/BUILD.gn#24)
+From //build/BUILD.gn:24
**Overridden from the default:**` + " `4`" + `
-From [//base/BUILD.gn:2](http://fuchsia.com/base/BUILD.gn#2)
+From //base/BUILD.gn:2
`
@@ -233,7 +228,7 @@
func TestUnique(t *testing.T) {
gnArgs := []Arg{x64Arg, arm64Arg}
- argMap := NewArgMap(Sources())
+ argMap := NewArgMap()
for _, arg := range gnArgs {
argMap.AddArg(arg)
}
@@ -248,6 +243,7 @@
## ` + "`target_cpu = arm64`" + `
### arm64
+
Description of arm64 arg.
**Current value for ` + "`target_cpu = arm64`:** `arg`" + `
@@ -257,7 +253,8 @@
## ` + "`target_cpu = x64`" + `
### x64
-Description of x64 arg that references [//build/path.py](http://fuchsia.com/build/path.py), //sources, and [//base](http://fuchsia.com/base).
+
+Description of x64 arg that references //build/path.py, //sources, and //base.
**Current value for ` + "`target_cpu = x64`:** `1`" + `
@@ -272,7 +269,7 @@
func TestTwoKeys(t *testing.T) {
gnArgs := []Arg{twoKeyarm64TestArg, twoKeyx64TestArg, twoKeyarm64OtherArg, twoKeyx64OtherArg}
- argMap := NewArgMap(Sources())
+ argMap := NewArgMap()
for _, arg := range gnArgs {
argMap.AddArg(arg)
}
@@ -287,6 +284,7 @@
## ` + "`target_cpu = arm64, package='other/package/default'`" + `
### arm64Other
+
Description of arm64 arg.
**Current value for ` + "`target_cpu = arm64, package='other/package/default'`:** `arg`" + `
@@ -296,6 +294,7 @@
## ` + "`target_cpu = arm64, package='test/package/default'`" + `
### arm64
+
Description of arm64 arg.
**Current value for ` + "`target_cpu = arm64, package='test/package/default'`:** `arg`" + `
@@ -305,6 +304,7 @@
## ` + "`target_cpu = x64, package='other/package/default'`" + `
### x64Other
+
Description of x64 arg.
**Current value for ` + "`target_cpu = x64, package='other/package/default'`:** `arg`" + `
@@ -314,6 +314,7 @@
## ` + "`target_cpu = x64, package='test/package/default'`" + `
### x64
+
Description of x64 arg.
**Current value for ` + "`target_cpu = x64, package='test/package/default'`:** `arg`" + `
@@ -329,7 +330,7 @@
func TestValueNewLine(t *testing.T) {
gnArgs := []Arg{newLineValueArg}
- argMap := NewArgMap(Sources())
+ argMap := NewArgMap()
for _, arg := range gnArgs {
argMap.AddArg(arg)
}
@@ -344,10 +345,11 @@
## All builds
### NewLine
+
Description of newline arg.
**Current value (from the default):**
-` + "```" + `
+` + "\n```none" + `
{
base = "//build/toolchain/fuchsia:x64"
}
diff --git a/gndoc/sourceparser.go b/gndoc/sourceparser.go
deleted file mode 100644
index 79a5bc2..0000000
--- a/gndoc/sourceparser.go
+++ /dev/null
@@ -1,64 +0,0 @@
-// Copyright 2018 The Fuchsia Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-package gndoc
-
-import (
- "encoding/json"
- "fmt"
- "io"
- "path/filepath"
- "strings"
-)
-
-type SourceMap map[string]string
-
-func NewSourceMap(input io.Reader) (*SourceMap, error) {
- s := SourceMap(make(map[string]string))
- err := s.parseJson(input)
- if err != nil {
- return nil, err
- }
- return &s, nil
-}
-
-// parseJson extracts the source project information into the map.
-func (s SourceMap) parseJson(input io.Reader) error {
- // Decode the json into the map.
- var sources []map[string]interface{}
- decoder := json.NewDecoder(input)
- if err := decoder.Decode(&sources); err != nil {
- return err
- }
-
- for _, source := range sources {
- if revision, ok := source["revision"]; ok {
- s[source["name"].(string)] = fmt.Sprintf("%v/+/%s", source["remote"], revision)
- } else {
- s[source["name"].(string)] = fmt.Sprintf("%v/+/master", source["remote"])
- }
- }
-
- return nil
-}
-
-// GetSourceLink returns a URL based on a file and a line number.
-func (s SourceMap) GetSourceLink(file string, line int) string {
- // We need to trim off first slashes in the filename.
- file = strings.TrimPrefix(file, "//")
- project := file
- for {
- if _, exists := s[project]; exists {
- break
- }
- project = filepath.Dir(project)
- if project == "." || project == "/" || project == "" {
- return ""
- }
- }
- if line == 0 {
- return fmt.Sprintf("%s%s", s[project], file[len(project):])
- }
- return fmt.Sprintf("%s%s#%d", s[project], file[len(project):], line)
-}
diff --git a/gndoc/sourceparser_test.go b/gndoc/sourceparser_test.go
deleted file mode 100644
index 420c531..0000000
--- a/gndoc/sourceparser_test.go
+++ /dev/null
@@ -1,83 +0,0 @@
-// Copyright 2018 The Fuchsia Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-package gndoc
-
-import (
- "strings"
- "testing"
-)
-
-func TestNewSourceMap(t *testing.T) {
- sources := `[
- {
- "name": "build",
- "path": "/path/to/fuchsia/build",
- "relativePath": "build",
- "remote": "https://fuchsia.googlesource.com/build",
- "revision": "43d2b55675d428d460fe6f91092bbf3c39552caf",
- "branches": [
- "mybranch",
- "master"
- ]
- },
- {
- "name": "buildtools",
- "path": "/path/to/fuchsia/buildtools",
- "relativePath": "buildtools",
- "remote": "https://fuchsia.googlesource.com/buildtools"
- }
-]
-`
- actual, err := NewSourceMap(strings.NewReader(sources))
- if err != nil {
- t.Fatalf("In TestNewSourceMap, unable to create source map: %s", err)
- }
-
- expected := SourceMap(make(map[string]string))
- expected["build"] = "https://fuchsia.googlesource.com/build/+/43d2b55675d428d460fe6f91092bbf3c39552caf"
- expected["buildtools"] = "https://fuchsia.googlesource.com/buildtools/+/master"
-
- if len(expected) != len(*actual) {
- t.Fatalf("In TestNewSourceMap, expected \n%d but got \n%d", len(expected), len(*actual))
- }
-
- if expected["build"] != (*actual)["build"] {
- t.Fatalf("In TestNewSourceMap, expected \n%s but got \n%s", expected["build"], (*actual)["build"])
- }
-
- if expected["buildtools"] != (*actual)["buildtools"] {
- t.Fatalf("In TestNewSourceMap, expected \n%s but got \n%s", expected["buildtools"], (*actual)["buildtools"])
- }
-}
-
-func TestGetSourceLink(t *testing.T) {
- sourceMap := SourceMap(make(map[string]string))
- sourceMap["build"] = "https://fuchsia.googlesource.com/build/+/43d2b55675d428d460fe6f91092bbf3c39552caf"
- sourceMap["buildtools"] = "https://fuchsia.googlesource.com/buildtools/+/master"
-
- expected := "https://fuchsia.googlesource.com/build/+/43d2b55675d428d460fe6f91092bbf3c39552caf/BUILD.gn#27"
- actual := sourceMap.GetSourceLink("//build/BUILD.gn", 27)
- if expected != actual {
- t.Fatalf("In TestNewSourceMap, expected \n%s but got \n%s", expected, actual)
- }
-
- expected = "https://fuchsia.googlesource.com/build/+/43d2b55675d428d460fe6f91092bbf3c39552caf/BUILD.gn"
- actual = sourceMap.GetSourceLink("//build/BUILD.gn", 0)
- if expected != actual {
- t.Fatalf("In TestNewSourceMap, expected \n%s but got \n%s", expected, actual)
- }
-
- expected = ""
- actual = sourceMap.GetSourceLink("//base/BUILD.gn", 0)
- if expected != actual {
- t.Fatalf("In TestNewSourceMap, expected \n%s but got \n%s", expected, actual)
- }
-
- expected = ""
- actual = sourceMap.GetSourceLink("/home/user/fuchsia/out/default.zircon/args.gn", 5)
- if expected != actual {
- t.Fatalf("In TestNewSourceMap, expected \n%s but got \n%s", expected, actual)
- }
-}
