Fix recursive download and dependency ordering

This involves rewriting substantial potions of
the pacman/graph package.

This resolves issue #33.

Author: Ben Morgan

NEWS.md

@@ -4,10 +4,12 @@ Repoctl Releases
 ## Version 0.20 (unreleased)
 
 - New: `search` command added.
+- New `down` command learned `-n` (`--dry-run`) option.
 - Update: `new config` command now backs up existing configuration files.
 - Update: `github.com/goulash/pacman` dependency moved into repository.
 - Bugfix: `version` command does not show entire configuration.
 - Bugfix: issue #46, do not panic or print errors with large repos.
+- Bugfix: issue #33, recursive download and dependency resolution broken.
 
 ## Version 0.19 (25 October 2019)
 This release fixes several bugs and adds support for signatures and Zst

cmd/repoctl/down.go

@@ -8,13 +8,15 @@ import (
 	"fmt"
 	"os"
 
+	"github.com/cassava/repoctl/pacman/aur"
 	"github.com/cassava/repoctl/pacman/graph"
 	"github.com/cassava/repoctl/pacman/pkgutil"
 	"github.com/spf13/cobra"
 )
 
 var (
 	downDest     string
+	downDryRun   bool
 	downClobber  bool
 	downExtract  bool
 	downUpgrades bool
@@ -27,6 +29,7 @@ func init() {
 	MainCmd.AddCommand(downCmd)
 
 	downCmd.Flags().StringVarP(&downDest, "dest", "d", "", "output directory for tarballs")
+	downCmd.Flags().BoolVarP(&downDryRun, "dry-run", "n", false, "don't download any packages")
 	downCmd.Flags().BoolVarP(&downClobber, "clobber", "l", false, "delete conflicting files and folders")
 	downCmd.Flags().BoolVarP(&downExtract, "extract", "e", true, "extract the downloaded tarballs")
 	downCmd.Flags().BoolVarP(&downUpgrades, "upgrades", "u", false, "download tarballs for all upgrades")
@@ -40,13 +43,38 @@ var downCmd = &cobra.Command{
 	Aliases: []string{"download"},
 	Short:   "download and extract tarballs from AUR",
 	Long: `Download and extract tarballs from AUR for given packages.
-Alternatively, all packages, or those with updates can be downloaded.
-Options specified are additive, not exclusive.
 
-By default, tarballs are deleted after being extracted, and are placed
-in the current directory.
+  Alternatively, all packages, or those with updates can be downloaded.
+  Options specified are additive, not exclusive.
+
+  By default, tarballs are deleted after being extracted, and are placed
+  in the current directory.
+
+  Packages can also be downloaded recursively, and the list that these
+  dependencies should be built can be saved. For example, to download
+  all updates to the repository and build them in approximately the
+  correct order:
+
+    repoctl down -o build-order.txt -u
+    for pkg in $(cat build-order.txt); do
+        (
+            cd $pkg
+            makepkg -si
+            ok=$?
+            if $ok; then
+                repoctl add -m *.pkg.tar*
+                cd ..
+                rm -rf $pkg
+            fi
+        )
+    done
+
+  Caveat: Automatic dependency resolution does not currently handle version
+  resolution or library specifications, as noted in the Arch wiki at:
+  https://wiki.archlinux.org/index.php/PKGBUILD#Dependencies
 `,
-	Example: `  repoctl down -u`,
+	Example: `  repoctl down -u
+  repoctl down -o build-order.txt -u`,
 	RunE: func(cmd *cobra.Command, args []string) error {
 		// First, populate the initial list of packages to download.
 		var list []string
@@ -70,28 +98,51 @@ in the current directory.
 
 		// If no dependencies are wanted, then get to it right away:
 		if !downRecurse && downOrder == "" {
+			// There's not much point to a try run here, but we should respect
+			// the option nevertheless.
+			if downDryRun {
+				return nil
+			}
 			return Repo.Download(nil, downDest, downExtract, downClobber, list...)
 		}
 
-		// Otherwise, get the dependencies:
-		g, err := Repo.DependencyGraph(nil, list...)
+		// Otherwise, get the dependency list and download the packages:
+		aps, err := downDependencies(list)
 		if err != nil {
 			return err
 		}
-		_, aps, ups := graph.Dependencies(g)
-		if downOrder != "" {
-			f, err := os.Create(downOrder)
-			if err != nil {
-				return err
-			}
-			for _, p := range aps {
-				fmt.Fprintln(f, p.Name)
-			}
-			f.Close()
-		}
-		for _, u := range ups {
-			fmt.Fprintf(os.Stderr, "warning: unknown package %s\n", u)
+		// Don't download any packages if dry run is activated.
+		if downDryRun {
+			return nil
 		}
 		return Repo.DownloadPackages(nil, aps, downDest, downExtract, downClobber)
 	},
 }
+
+func downDependencies(packages []string) (aur.Packages, error) {
+	g, err := Repo.DependencyGraph(nil, packages...)
+	if err != nil {
+		return nil, err
+	}
+	_, aps, ups := graph.Dependencies(g)
+	if downOrder != "" {
+		f, err := os.Create(downOrder)
+		if err != nil {
+			return nil, err
+		}
+
+		for i := len(aps); i != 0; i-- {
+			fmt.Fprintln(f, aps[i-1].Name)
+		}
+		f.Close()
+	}
+	for _, u := range ups {
+		fmt.Fprintf(os.Stderr, "warning: unknown package %s\n", u)
+		iter := g.To(g.NodeWithName(u).ID())
+		for iter.Next() {
+			node := iter.Node().(*graph.Node)
+			fmt.Fprintf(os.Stderr, "         required by: %s\n", node.PkgName())
+		}
+	}
+	return aps, nil
+}

pacman/graph/factory.go

@@ -2,10 +2,11 @@ package graph
 
 import (
 	"os"
+	"regexp"
 
-	"github.com/goulash/errs"
 	"github.com/cassava/repoctl/pacman"
 	"github.com/cassava/repoctl/pacman/aur"
+	"github.com/goulash/errs"
 )
 
 // A Factory creates a dependency graph given a set of packages.
@@ -38,6 +39,7 @@ type Factory struct {
 // as a leaf in the graph, since we assume that pacman can resolve those
 // dependencies.
 func NewFactory(ignoreRepos ...string) (*Factory, error) {
+	re := regexp.MustCompile(`(=|>|<).*$`)
 	f := Factory{
 		skipInstalled: false,
 		truncate:      false,
@@ -46,6 +48,16 @@ func NewFactory(ignoreRepos ...string) (*Factory, error) {
 			deps := make([]string, 0, len(p.PkgDepends())+len(p.PkgMakeDepends()))
 			deps = append(deps, p.PkgDepends()...)
 			deps = append(deps, p.PkgMakeDepends()...)
+			for i, p := range deps {
+				// Quote from: https://wiki.archlinux.org/index.php/PKGBUILD
+				// > Version restrictions can be specified with comparison
+				// > operators, e.g. depends=('foobar>=1.8.0'); if multiple
+				// > restrictions are needed, the dependency can be repeated for
+				// > each, e.g. depends=('foobar>=1.8.0' 'foobar<2.0.0').
+				if re.MatchString(p) {
+					deps[i] = re.ReplaceAllLiteralString(p, "")
+				}
+			}
 			return deps
 		},
 	}
@@ -132,8 +144,8 @@ func (f *Factory) NewGraph(pkgs aur.Packages) (*Graph, error) {
 	}
 
 	// As long as we have new packages to process, continue.
-	for len(lst) == 0 {
-		new := make([]*Node, 0)
+	for len(lst) != 0 {
+		discovered := make([]*Node, 0)
 		unavailable := make(map[string]bool, 0)
 		pending := make(map[string][]*Node)
 
@@ -157,8 +169,9 @@ func (f *Factory) NewGraph(pkgs aur.Packages) (*Graph, error) {
 					u := g.NewNode(p)
 					if !f.truncate {
 						// Process this package for dependencies
-						new = append(new, u)
+						discovered = append(discovered, u)
 					}
+					g.AddNode(u)
 					g.AddEdgeFromTo(v, u)
 					continue
 				}
@@ -172,17 +185,17 @@ func (f *Factory) NewGraph(pkgs aur.Packages) (*Graph, error) {
 		}
 
 		// This may be called for AUR or unknown packages
-		addFetchedPkg := func(p pacman.AnyPackage) {
+		addFetchedPkg := func(p pacman.AnyPackage) *Node {
 			u := g.NewNode(p)
-			new = append(new, u)
 			g.AddNode(u)
 			for _, v := range pending[u.PkgName()] {
 				g.AddEdgeFromTo(v, u)
 			}
+			return u
 		}
 
 		// Get all unavailable packages from AUR:
-		fromAUR := make([]string, len(unavailable))
+		fromAUR := make([]string, 0, len(unavailable))
 		for k := range unavailable {
 			fromAUR = append(fromAUR, k)
 		}
@@ -207,10 +220,11 @@ func (f *Factory) NewGraph(pkgs aur.Packages) (*Graph, error) {
 			}
 		}
 		for _, p := range pkgs {
-			addFetchedPkg(p)
+			u := addFetchedPkg(p)
+			discovered = append(discovered, u)
 		}
 
-		lst = new
+		lst = discovered
 	}
 
 	return g, nil

pacman/graph/functions.go

@@ -1,10 +1,10 @@
 package graph
 
 import (
-	"github.com/gonum/graph"
-	"github.com/gonum/graph/traverse"
 	"github.com/cassava/repoctl/pacman"
 	"github.com/cassava/repoctl/pacman/aur"
+
+	"gonum.org/v1/gonum/graph/topo"
 )
 
 // Dependencies returns a list of all dependencies in the graph,
@@ -15,7 +15,7 @@ func Dependencies(g *Graph) (pacman.Packages, aur.Packages, []string) {
 	ups := make([]string, 0)
 
 	names := make(map[string]bool)
-	nodes := AllNodesBottomUp(g)
+	nodes, _ := topo.Sort(g)
 	for _, vn := range nodes {
 		n := vn.(*Node)
 		if names[n.PkgName()] {
@@ -38,69 +38,3 @@ func Dependencies(g *Graph) (pacman.Packages, aur.Packages, []string) {
 	}
 	return rps, aps, ups
 }
-
-// Roots returns all the root nodes for a directed graph.
-func Roots(g graph.Directed) []graph.Node {
-	roots := make([]graph.Node, 0)
-	for _, n := range g.Nodes() {
-		if g.To(n) == nil || len(g.To(n)) == 0 {
-			roots = append(roots, n)
-		}
-	}
-	return roots
-}
-
-// NodesBottomUp returns the subtree from the bottom levels upwards to the root.
-// The nodes may appear multiple times however.
-func NodesBottomUp(g graph.Directed, root graph.Node) []graph.Node {
-	nodes := make([]graph.Node, 0)
-	nodes = append(nodes, root)
-	bfs := traverse.BreadthFirst{}
-	bfs.Walk(g, root, func(v graph.Node, _ int) bool {
-		nodes = append(nodes, v)
-		return true
-	})
-
-	// Reverse the list
-	sz := len(nodes)
-	last := sz - 1
-	for i := 0; i < sz/2; i++ {
-		tmp := nodes[i]
-		nodes[i] = nodes[last-i]
-		nodes[last-i] = tmp
-	}
-	return uniqueNodes(nodes)
-}
-
-// AllNodesBottomUp returns for all roots the nodes bottom-up.
-func AllNodesBottomUp(g graph.Directed) []graph.Node {
-	nodes := make([]graph.Node, 0)
-	for _, root := range Roots(g) {
-		nodes = append(nodes, NodesBottomUp(g, root)...)
-	}
-	return uniqueNodes(nodes)
-}
-
-func uniqueStrings(list []string) []string {
-	xs := make(map[string]bool)
-	lst := make([]string, 0, len(list))
-	for _, x := range list {
-		if !xs[x] {
-			xs[x] = true
-			lst = append(lst, x)
-		}
-	}
-	return lst
-}
-
-func uniqueNodes(list []graph.Node) []graph.Node {
-	xs := make(map[int]bool)
-	lst := make([]graph.Node, 0, len(list))
-	for _, x := range list {
-		if !xs[x.ID()] {
-			xs[x.ID()] = true
-			lst = append(lst, x)
-		}
-	}
-	return lst
-}