Skip to content

Commit

Permalink
Feature: Add --probSpecsDir option
Browse files Browse the repository at this point in the history
The user can now tell the program to use an existing
`problem-specifications` directory, in which case the program does a
`fetch` rather than making a temporary shallow clone.

This is one of the biggest possible speed optimisations, given that the
program running time in non-interactive mode is limited by the time
taken to establish an up-to-date `problem-specifications`directory.
However, in the future we could add an `--offline` option (or similar).

This commit also makes it easier to add tests.

Decisions:
- Show an error if the given `problem-specifications` working directory
  is not clean, rather than asking the user or stashing changes.
- Try to be strict about the user's `problem-specifications` directory,
  and do not silently `checkout` or `merge`, at least for now.
- Don't assume the user has a remote named 'upstream'.
- Allow the given `problem-specifications` repo to be a shallow clone:
  don't check that it's actually a `problem-specifications` repo by
  requiring it to contain some specified SHA-1.

Closes: exercism#47
  • Loading branch information
ee7 committed Oct 23, 2020
1 parent fefd189 commit d5d193d
Show file tree
Hide file tree
Showing 2 changed files with 93 additions and 10 deletions.
11 changes: 9 additions & 2 deletions src/cli.nim
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,11 @@ type
exercise*: Option[string]
mode*: Mode
verbosity*: Verbosity
probSpecsDir*: Option[string]

Opt = enum
optExercise, optCheck, optMode, optVerbosity, optHelp, optVersion
optExercise, optCheck, optMode, optVerbosity, optProbSpecsDir,
optHelp, optVersion

OptKey = tuple
short: string
Expand All @@ -32,6 +34,7 @@ const
("c", "check"),
("m", "mode"),
("o", "verbosity"),
("p", "probSpecsDir"),
("h", "help"),
("v", "version"),
]
Expand All @@ -54,6 +57,7 @@ Options:
-{optCheck.short}, --{optCheck.long} Check if there are missing tests. Doesn't update the tests. Terminates with a non-zero exit code if one or more tests are missing
-{optMode.short}, --{optMode.long} <mode> What to do with missing test cases. Allowed values: c[hoose], i[nclude], e[xclude]
-{optVerbosity.short}, --{optVerbosity.long} <verbosity> The verbosity of output. Allowed values: q[uiet], n[ormal], d[etailed]
-{optProbSpecsDir.short}, --{optProbSpecsDir.long} <dir> Use this `problem-specifications` directory, rather than cloning temporarily
-{optHelp.short}, --{optHelp.long} Show this help message and exit
-{optVersion.short}, --{optVersion.long} Show this tool's version information and exit"""

Expand All @@ -63,7 +67,7 @@ proc showVersion =
echo &"Canonical Data Syncer v{NimblePkgVersion}"
quit(0)

proc showError(s: string) =
proc showError*(s: string) =
stdout.styledWrite(fgRed, "Error: ")
stdout.write(s)
stdout.write("\n\n")
Expand Down Expand Up @@ -134,6 +138,9 @@ proc processCmdLine*: Conf =
of optVerbosity.short, optVerbosity.long:
showErrorForMissingVal(kind, key, val)
result.verbosity = parseVerbosity(kind, key, val)
of optProbSpecsDir.short, optProbSpecsDir.long:
showErrorForMissingVal(kind, key, val)
result.probSpecsDir = some(val)
of optHelp.short, optHelp.long:
showHelp()
of optVersion.short, optVersion.long:
Expand Down
92 changes: 84 additions & 8 deletions src/probspecs.nim
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import std/[json, options, os, osproc, sequtils, strformat, strutils]
import std/[json, options, os, osproc, sequtils, strformat, strscans, strutils]
import cli, logger

type
Expand Down Expand Up @@ -102,12 +102,88 @@ proc findProbSpecsExercises(repo: ProbSpecsRepo, conf: Conf): seq[ProbSpecsExerc
if conf.exercise.isNone or conf.exercise.get() == repoExercise.slug:
result.add(initProbSpecsExercise(repoExercise))

proc findProbSpecsExercises*(conf: Conf): seq[ProbSpecsExercise] =
let probSpecsRepo = initProbSpecsRepo()

template withDir(dir: string; body: untyped): untyped =
## Changes the current directory to `dir` temporarily.
let startDir = getCurrentDir()
try:
probSpecsRepo.remove()
probSpecsRepo.clone()
probSpecsRepo.findProbSpecsExercises(conf)
setCurrentDir(dir)
body
finally:
probSpecsRepo.remove()
setCurrentDir(startDir)

proc getNameOfRemote(probSpecsDir, location: string): string =
## Returns the name of the remote in `probSpecsDir` that points to `location`.
##
## Raises an error if there is no remote that points to `location`.
# There's probably a better way to do this than parsing `git remote -v`.
let (remotes, errRemotes) = execCmdEx("git remote -v")
if errRemotes != 0:
showError("could not run `git remote -v` in the given " &
&"problem-specifications directory: '{probSpecsDir}'")
var remoteName, remoteUrl: string
for line in remotes.splitLines():
discard line.scanf("$s$w$s$+fetch)$.", remoteName, remoteUrl)
if remoteUrl.contains(location):
return remoteName
showError(&"there is no remote that points to '{location}' in the " &
&"given problem-specifications directory: '{probSpecsDir}'")

proc validateProbSpecsRepo(probSpecsRepo: ProbSpecsRepo) =
## Raises an error if the given `probSpecsRepo` is not a valid
## `problem-specifications` repo that is up-to-date with upstream.
const mainBranchName = "master"

let probSpecsDir = probSpecsRepo.dir
logDetailed(&"Using user-provided problem-specifications dir: {probSpecsDir}")

# Exit if the given directory does not exist.
if not dirExists(probSpecsDir):
showError("the given problem-specifications directory does not exist: " &
&"'{probSpecsDir}'")

withDir probSpecsDir:
# Exit if the given directory is not a git repo.
if execCmd("git rev-parse") != 0:
showError("the given problem-specifications directory is not a git " &
&"repository: '{probSpecsDir}'")

# Exit if the working directory is not clean.
if execCmd("git diff-index --quiet HEAD") != 0: # Ignores untracked files.
echo &"\nUnstaged changes in {probSpecsDir}:"
discard execCmd("git status --short --untracked=no")
showError("the given problem-specifications working directory is not " &
&"clean: '{probSpecsDir}'")

# Find the name of the remote that points to upstream. Don't assume the
# remote is called 'upstream'.
# Exit if the repo has no remote that points to upstream.
const upstreamLocation = "github.com/exercism/problem-specifications"
let remoteName = getNameOfRemote(probSpecsDir, upstreamLocation)

# For now, just exit with an error if the HEAD is not up-to-date with
# upstream, even if it's possible to do a fast-forward merge.
if execCmd(&"git fetch --quiet {remoteName} {mainBranchName}") != 0:
showError(&"failed to fetch `{mainBranchName}` in " &
&"problem-specifications directory: '{probSpecsDir}'")

# Allow HEAD to be on a non-`master` branch, as long as it's up-to-date
# with `upstream/master`.
let (revHead, _) = execCmdEx("git rev-parse HEAD")
let (revUpstream, _) = execCmdEx(&"git rev-parse {remoteName}/{mainBranchName}")
if revHead != revUpstream:
showError("the given problem-specifications directory is not " &
&"up-to-date: '{probSpecsDir}'")

proc findProbSpecsExercises*(conf: Conf): seq[ProbSpecsExercise] =
if conf.probSpecsDir.isSome():
let probSpecsRepo = ProbSpecsRepo(dir: conf.probSpecsDir.get())
validateProbSpecsRepo(probSpecsRepo)
result = probSpecsRepo.findProbSpecsExercises(conf)
else:
let probSpecsRepo = initProbSpecsRepo()
try:
probSpecsRepo.remove()
probSpecsRepo.clone()
result = probSpecsRepo.findProbSpecsExercises(conf)
finally:
probSpecsRepo.remove()

0 comments on commit d5d193d

Please sign in to comment.