Merge pull request #36 from github/vdye/derived-route

Make `route` optional in `git-bundle-server init`

Author: vdye

README.md

@@ -104,7 +104,7 @@ being managed by the bundle server.
   server route to find the data for this repository. Otherwise, the route is
   inferred from `<url>` by removing the domain name. For example,
   `https://github.com/git-for-windows/git` is assigned the route
-  `/git-for-windows/git`. Run `git-bundle-server update` to initialize bundle
+  `git-for-windows/git`. Run `git-bundle-server update` to initialize bundle
   information. Configure the web server to recognize this repository at that
   route. Configure scheduler to run `git-bundle-server update-all` as
   necessary.

cmd/git-bundle-server/delete.go

@@ -34,7 +34,7 @@ data.`
 
 func (d *deleteCmd) Run(ctx context.Context, args []string) error {
 	parser := argparse.NewArgParser(d.logger, "git-bundle-server delete <route>")
-	route := parser.PositionalString("route", "the route to delete")
+	route := parser.PositionalString("route", "the route to delete", true)
 	parser.Parse(ctx, args)
 
 	repoProvider := utils.GetDependency[core.RepositoryProvider](ctx, d.container)

cmd/git-bundle-server/init.go

@@ -35,12 +35,20 @@ should be hosted at '<route>'.`
 }
 
 func (i *initCmd) Run(ctx context.Context, args []string) error {
-	parser := argparse.NewArgParser(i.logger, "git-bundle-server init <url> <route>")
-	url := parser.PositionalString("url", "the URL of a repository to clone")
-	// TODO: allow parsing <route> out of <url>
-	route := parser.PositionalString("route", "the route to host the specified repo")
+	parser := argparse.NewArgParser(i.logger, "git-bundle-server init <url> [<route>]")
+	url := parser.PositionalString("url", "the URL of a repository to clone", true)
+	route := parser.PositionalString("route", "the route to host the specified repo", false)
 	parser.Parse(ctx, args)
 
+	// Set route value, if needed
+	if *route == "" {
+		var ok bool
+		*route, ok = core.GetRouteFromUrl(*url)
+		if !ok {
+			parser.Usage(ctx, "Cannot parse route from url '%s'; please specify an explicit route.", *url)
+		}
+	}
+
 	repoProvider := utils.GetDependency[core.RepositoryProvider](ctx, i.container)
 	bundleProvider := utils.GetDependency[bundles.BundleProvider](ctx, i.container)
 	gitHelper := utils.GetDependency[git.GitHelper](ctx, i.container)

cmd/git-bundle-server/start.go

@@ -34,7 +34,7 @@ specified '<route>'.`
 
 func (s *startCmd) Run(ctx context.Context, args []string) error {
 	parser := argparse.NewArgParser(s.logger, "git-bundle-server start <route>")
-	route := parser.PositionalString("route", "the route for which bundles should be generated")
+	route := parser.PositionalString("route", "the route for which bundles should be generated", true)
 	parser.Parse(ctx, args)
 
 	repoProvider := utils.GetDependency[core.RepositoryProvider](ctx, s.container)

cmd/git-bundle-server/stop.go

@@ -33,7 +33,7 @@ specified '<route>'.`
 
 func (s *stopCmd) Run(ctx context.Context, args []string) error {
 	parser := argparse.NewArgParser(s.logger, "git-bundle-server stop <route>")
-	route := parser.PositionalString("route", "the route for which bundles should stop being generated")
+	route := parser.PositionalString("route", "the route for which bundles should stop being generated", true)
 	parser.Parse(ctx, args)
 
 	repoProvider := utils.GetDependency[core.RepositoryProvider](ctx, s.container)

cmd/git-bundle-server/update.go

@@ -36,7 +36,7 @@ bundles, and update the bundle list.`
 
 func (u *updateCmd) Run(ctx context.Context, args []string) error {
 	parser := argparse.NewArgParser(u.logger, "git-bundle-server update <route>")
-	route := parser.PositionalString("route", "the route to update")
+	route := parser.PositionalString("route", "the route to update", true)
 	parser.Parse(ctx, args)
 
 	repoProvider := utils.GetDependency[core.RepositoryProvider](ctx, u.container)

docs/man/git-bundle-server.adoc

@@ -53,15 +53,17 @@ the server.
 
 == COMMANDS
 
-*init* _url_ _route_::
+*init* _url_ [_route_]::
   Initialize a repository for which bundles should be served. The repository is
   cloned into a bare repo from _url_. A base bundle is created for the
-  repository, served from _route_, and the man:cron[8] global bundle update
-  schedule is started.
+  repository and used to initialize the bundle list. If _route_ is specified,
+  the bundle list will be served from that route; otherwise, the route is
+  derived from the _url_. Finally, the man:cron[8] global bundle update schedule
+  is started.
 +
-It is recommended that users specify an SSH (rather than HTTP) URL for the
-_url_ argument to avoid potentially error-causing authentication prompts
-while fetching during scheduled bundle updates.
+It is recommended that users specify an SSH (rather than HTTP) URL for the _url_
+argument to avoid potentially error-causing authentication prompts while
+fetching during scheduled bundle updates.
 
 *start* _route_::
   Start computing bundles for the repository identified by _route_. If the

internal/argparse/argparse.go

@@ -15,6 +15,7 @@ const usageExitCode int = 2
 type positionalArg struct {
 	name        string
 	description string
+	required    bool
 	value       interface{}
 }
 
@@ -60,7 +61,7 @@ func NewArgParser(logger log.TraceLogger, usageString string) *argParser {
 			fmt.Fprint(out, "\n")
 		}
 
-		// Print subcommands (if any)
+		// Print subcommands or positional args (if any)
 		if len(a.subcommands) > 0 {
 			if a.isTopLevel {
 				fmt.Fprintln(out, "Commands:")
@@ -69,6 +70,10 @@ func NewArgParser(logger log.TraceLogger, usageString string) *argParser {
 			}
 			a.printSubcommands()
 			fmt.Fprint(out, "\n")
+		} else if len(a.positionalArgs) > 0 {
+			fmt.Fprintln(out, "Positional arguments:")
+			a.printPositionalArgs()
+			fmt.Fprint(out, "\n")
 		}
 	}
 
@@ -93,31 +98,48 @@ func (a *argParser) Subcommand(subcommand Subcommand) {
 	a.subcommands[subcommand.Name()] = subcommand
 }
 
-func (a *argParser) PositionalStringVar(name string, description string, arg *string) {
+func (a *argParser) printPositionalArgs() {
+	out := a.FlagSet.Output()
+	for _, arg := range a.positionalArgs {
+		optionalStr := ""
+		if !arg.required {
+			optionalStr = "(optional) "
+		}
+		fmt.Fprintf(out, "  %s\n    \t%s%s\n",
+			arg.name,
+			optionalStr,
+			strings.ReplaceAll(strings.TrimSpace(arg.description), "\n", "\n    \t"),
+		)
+	}
+}
+
+func (a *argParser) PositionalStringVar(name string, description string, arg *string, required bool) {
 	a.positionalArgs = append(a.positionalArgs, &positionalArg{
 		name:        name,
 		description: description,
+		required:    required,
 		value:       arg,
 	})
 }
 
-func (a *argParser) PositionalString(name string, description string) *string {
+func (a *argParser) PositionalString(name string, description string, required bool) *string {
 	arg := new(string)
-	a.PositionalStringVar(name, description, arg)
+	a.PositionalStringVar(name, description, arg, required)
 	return arg
 }
 
-func (a *argParser) PositionalListVar(name string, description string, arg *[]string) {
+func (a *argParser) PositionalListVar(name string, description string, arg *[]string, required bool) {
 	a.positionalArgs = append(a.positionalArgs, &positionalArg{
 		name:        name,
 		description: description,
+		required:    required,
 		value:       arg,
 	})
 }
 
-func (a *argParser) PositionalList(name string, description string) *[]string {
+func (a *argParser) PositionalList(name string, description string, required bool) *[]string {
 	arg := &[]string{}
-	a.PositionalListVar(name, description, arg)
+	a.PositionalListVar(name, description, arg, required)
 	return arg
 }
 
@@ -131,7 +153,14 @@ func (a *argParser) Parse(ctx context.Context, args []string) {
 	if len(a.subcommands) > 0 && len(a.positionalArgs) > 0 {
 		panic("cannot mix subcommands and positional args")
 	}
+
+	mustBeOptional := false
 	for i, positionalArg := range a.positionalArgs {
+		mustBeOptional = mustBeOptional || !positionalArg.required
+		if mustBeOptional && positionalArg.required {
+			panic("cannot have required args after optional args")
+		}
+
 		if i < len(a.positionalArgs)-1 {
 			// Only the last positional arg can be a list
 			_, isList := positionalArg.value.(*[]string)
@@ -165,6 +194,14 @@ func (a *argParser) Parse(ctx context.Context, args []string) {
 	} else {
 		// Handle positional args
 		for _, arg := range a.positionalArgs {
+			if a.NArg() == 0 {
+				if arg.required {
+					a.Usage(ctx, "No value specified for required argument '%s'", arg.name)
+				} else {
+					break
+				}
+			}
+
 			// First, try single string case
 			sPtr, isStr := arg.value.(*string)
 			if isStr {

internal/core/funcs.go

@@ -0,0 +1,29 @@
+package core
+
+import (
+	"regexp"
+	"strings"
+)
+
+// Standalone helper functions for core (repo, cron, etc.) functionality.
+
+func GetRouteFromUrl(url string) (string, bool) {
+	matchers := []*regexp.Regexp{
+		// SSH, matches <username>@<domain>:<route>[.git]
+		regexp.MustCompile(`^[\w-]+@[\w\.-]+:([\w\.-]+/[\w\.-]+)/*$`),
+
+		// HTTP(S), matches http[s]://<domain>/<route>[.git]
+		regexp.MustCompile(`^(?i:http[s]?)://[\w\.-]+/([\w\.-]+/[\w\.-]+)/*$`),
+
+		// Filesystem, matches file://[<path>/]<route>[.git]
+		regexp.MustCompile(`^(?i:file)://[\w\.-/ ]*/([\w\.-]+/[\w\.-]+)/*$`),
+	}
+
+	for _, matcher := range matchers {
+		if groups := matcher.FindStringSubmatch(url); groups != nil {
+			return strings.TrimSuffix(groups[1], ".git"), true
+		}
+	}
+
+	return "", false
+}

internal/core/funcs_test.go

@@ -0,0 +1,122 @@
+package core_test
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/github/git-bundle-server/internal/core"
+	"github.com/stretchr/testify/assert"
+)
+
+var urlToRouteTests = []struct {
+	url           string
+	expectedRoute string
+	expectedMatch bool
+}{
+	// SSH tests
+	{
+		"[email protected]:git/git.git",
+		"git/git",
+		true,
+	},
+	{
+		"[email protected]:BUNDLE_SERVER.io/git-bundle-server",
+		"BUNDLE_SERVER.io/git-bundle-server",
+		true,
+	},
+	{
+		"[email protected]:imFineWith/trailingSlashes//",
+		"imFineWith/trailingSlashes",
+		true,
+	},
+	{
+		"test@mydomain:deeper/toodeep/cannotmatch",
+		"",
+		false,
+	},
+	{
+		"[email protected]:imFineWith/trailingSlashes/",
+		"imFineWith/trailingSlashes",
+		true,
+	},
+	{
+		"[email protected]:tooshallow",
+		"",
+		false,
+	},
+
+	// HTTP(S) tests
+	{
+		"hTTp://www.mysite.net/org/repo.git/",
+		"org/repo",
+		true,
+	},
+	{
+		"https://domain.test/clone/me",
+		"clone/me",
+		true,
+	},
+	{
+		"https://all.my.repos/having-some_fun/with.valid_ch4racters",
+		"having-some_fun/with.valid_ch4racters",
+		true,
+	},
+	{
+		"http://completely.normal.site/with/invalid/repo",
+		"",
+		false,
+	},
+	{
+		"HTTPS://SCREAM/INTOTHEVOID",
+		"",
+		false,
+	},
+
+	// Filesystem tests
+	{
+		"file:///root/path/to/a/repo.git",
+		"a/repo",
+		true,
+	},
+	{
+		"FILE://RELATIVE/to/me",
+		"to/me",
+		true,
+	},
+	{
+		"fIlE://spaces are allowed/in/path/to/repo",
+		"to/repo",
+		true,
+	},
+	{
+		"fIlE:///butspaces/are/NOT/allowed in route",
+		"",
+		false,
+	},
+	{
+		"file://somepathsaretooshort",
+		"",
+		false,
+	},
+}
+
+func TestGetRouteFromUrl(t *testing.T) {
+	for _, tt := range urlToRouteTests {
+		var title string
+		if tt.expectedMatch {
+			title = fmt.Sprintf("%s => %s", tt.url, tt.expectedRoute)
+		} else {
+			title = fmt.Sprintf("%s (no match)", tt.url)
+		}
+
+		t.Run(title, func(t *testing.T) {
+			route, isMatched := core.GetRouteFromUrl(tt.url)
+			if tt.expectedMatch {
+				assert.True(t, isMatched)
+				assert.Equal(t, tt.expectedRoute, route)
+			} else {
+				assert.False(t, isMatched, "Expected no match, got route %s", route)
+			}
+		})
+	}
+}