[GitHub] ccollins476ad commented on a change in pull request #266: `newt target dump` command

[GitBox <gi...@apache.org>]

ccollins476ad commented on a change in pull request #266: `newt target dump` command 
URL: https://github.com/apache/mynewt-newt/pull/266#discussion_r256530692
 
 

 ##########
 File path: newt/builder/depgraph.go
 ##########
 @@ -92,58 +91,86 @@ func revdepGraph(rs *resolve.ResolveSet) (DepGraph, error) {
 		return nil, err
 	}
 
-	revGm := graphMap{}
-	for parent, children := range graph {
-		// Make sure each node exists in the reverse graph.  This step is
-		// necessary for packages that have no dependers.
-		graphMapEnsure(revGm, parent)
-
-		// Add nodes for packages with with dependers.
-		for _, child := range children {
-			rParent := child.Rpkg
-			rChild := *child
-			rChild.Rpkg = parent
-			graphMapAdd(revGm, rParent, &rChild)
+	rgraph := DepGraph{}
+	for parent, entries := range graph {
+		// Ensure parent not is present in graph, even if no one depends on it.
+		if rgraph[parent] == nil {
+			rgraph[parent] = []DepEntry{}
+		}
+
+		// Reverse the dependency relationship for each child and add to the
+		// graph.
+		for _, entry := range entries {
+			rgraph[entry.PkgName] = append(rgraph[entry.PkgName], DepEntry{
+				PkgName:     parent,
+				DepExprs:    entry.DepExprs,
+				ReqApiExprs: entry.ReqApiExprs,
+				ApiExprs:    entry.ApiExprs,
+			})
 		}
 	}
 
-	return graphMapToDepGraph(revGm), nil
+	for _, entries := range rgraph {
+		SortDepEntries(entries)
+	}
+
+	return rgraph, nil
 }
 
-func depString(dep *resolve.ResolveDep) string {
-	s := fmt.Sprintf("%s", dep.Rpkg.Lpkg.FullName())
+func depString(entry DepEntry) string {
+	s := fmt.Sprintf("%s", entry.PkgName)
 
-	var reasons []string
-	if dep.Api != "" {
-		reasons = append(reasons, fmt.Sprintf("api:%s", dep.Api))
+	type ApiPair struct {
+		api   string
+		exprs []*parse.Node
 	}
 
-	exprStr := dep.ExprString()
-	if exprStr != "" {
-		reasons = append(reasons, fmt.Sprintf("syscfg:%s", exprStr))
-	}
+	if len(entry.ReqApiExprs) > 0 {
+		var apis []string
+		for api, _ := range entry.ReqApiExprs {
+			apis = append(apis, api)
+		}
+		sort.Strings(apis)
+
+		for _, api := range apis {
+			reqes := entry.ReqApiExprs[api]
+			reqdis := reqes.Disjunction().String()
+
+			apies := entry.ApiExprs[api]
+			apidis := apies.Disjunction().String()
 
-	if len(reasons) > 0 {
-		s += fmt.Sprintf("(%s)", strings.Join(reasons, " "))
+			s += "(api:" + api
+			if reqdis != "" || apidis != "" {
 
 Review comment:
   Yes, it does look a bit strange in this case.  As an example, when `sys/log/full`'s `log` API is made conditional on `OS_DEBUG_MODE`:
   ```
   pkg.apis.OS_DEBUG_MODE:
       - log
   ```
   
   then `newt revdep ...` produces the following:
   ```
   @apache-mynewt-core/sys/log/full <-- [@apache-mynewt-core/sys/log/modlog(api:log,syscfg:;OS_DEBUG_MODE)
   ```
   
   The semicolon looks weird, but at least the output is unambigous (the structure is always `<req-api-expr>;<api-expr>`.  I sort of gave up on presenting all this information in a "pretty" way, and settled for something that can be unambiguously translated back to source.  Suggestions for making this output more readable are much appreciated! :)

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services