Skip to content

Commit 1a295da

Browse files
authored
nimble: support patching multiple files and packages (#625)
We currently patch `cligen/parseopt3` at install time to improve the command-line parsing in an edge case. But we may want to patch further packages in the future, e.g. the cmark package for parsing markdown. Patching that package requires a different approach, as we need to patch a file outside the directory given by `nimble path cmark`. We must instead patch a file in the libcmark submodule before (re-)running `nimble install`, partly because Nimble doesn't copy a submodule into the installed package directory. Therefore, let's add a new proc to handle patching of Nimble packages, making sure that it supports patching: - a path that is not in the directory of any installed package. - multiple files (by running `git apply` in the root directory of the to-be-patched files). Other changes: - Move patching-related files to a new directory. - Add a `nimble path` helper, which can get paths to multiple packages and return an object (rather than a newline-delimited string). - Support a hash being greater than `int32.high`. - Remove a `fileExists` check. We already get an error message if the `readFile` fails, and it'd be better to use `try` / `except`. - Prefer to raise a `CatchableError`, rather than an `AssertionDefect`. - Remove some superfluous patching messages.
1 parent 5d98c51 commit 1a295da

File tree

3 files changed

+66
-34
lines changed

3 files changed

+66
-34
lines changed

configlet.nimble

Lines changed: 2 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import std/[hashes, os, strutils]
1+
import patches/patch
22

33
proc getVersionStart: string =
44
# Returns the `major.minor.patch` version in the `configlet.version` file
@@ -35,34 +35,5 @@ requires "isaac#45a5cbbd54ff59ba3ed94242620c818b9aad1b5b" # 0.1.3 (2017-11-
3535
task test, "Runs the test suite":
3636
exec "nim r ./tests/all_tests.nim"
3737

38-
# Patch `cligen/parseopt3` so that "--foo --bar" is parsed as two long options,
39-
# even when `longNoVal` is both non-empty and lacks `foo`.
4038
before build:
41-
# We want to support running `nimble build` before `cligen` is installed, so
42-
# we can't `import cligen/parseopt3` and check the parsing directly.
43-
# Instead, let's just hash the file and run `git apply` conditionally.
44-
# First, get the path to `parseopt3.nim` in the `cligen` package.
45-
let (output, exitCode) = gorgeEx("nimble path cligen")
46-
if exitCode == 0:
47-
let parseopt3Path = joinPath(output.strip(), "cligen", "parseopt3.nim")
48-
if fileExists(parseopt3Path):
49-
# Hash the file using `std/hashes`.
50-
# Note that we can't import `std/md5` or `std/sha1` in a .nimble file.
51-
let actualHash = parseopt3Path.readFile().hash()
52-
const patchedHash = 1647921161 # Update when bumping `cligen` changes `parseopt3`.
53-
if actualHash != patchedHash:
54-
echo "Trying to patch parseopt3..."
55-
echo "Found " & parseopt3Path
56-
let patchPath = thisDir() / "parseopt3_allow_long_option_optional_value.patch"
57-
let parseopt3Dir = parseopt3Path.parentDir()
58-
# Apply the patch.
59-
let cmd = "git -C " & parseopt3Dir & " apply --verbose " & patchPath
60-
let (outp, exitCode) = gorgeEx(cmd)
61-
echo outp
62-
if exitCode != 0:
63-
raise newException(AssertionDefect, "failed to apply patch")
64-
else:
65-
raise newException(AssertionDefect, "file does not exist: " & parseopt3Path)
66-
else:
67-
echo output
68-
raise newException(AssertionDefect, "failed to get cligen path")
39+
ensureThatNimblePackagesArePatched()

parseopt3_allow_long_option_optional_value.patch renamed to patches/parseopt3_allow_long_option_optional_value.patch

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
diff --git a/parseopt3.nim b/parseopt3.nim
1+
diff --git a/cligen/parseopt3.nim b/cligen/parseopt3.nim
22
index a865952..850d016 100644
3-
--- a/parseopt3.nim
4-
+++ b/parseopt3.nim
3+
--- a/cligen/parseopt3.nim
4+
+++ b/cligen/parseopt3.nim
55
@@ -267,8 +267,17 @@ proc doLong(p: var OptParser) =
66
p.kind = cmdError
77
return

patches/patch.nim

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
import std/[hashes, os, parseutils, strutils]
2+
3+
proc gorgeCheck(cmd, errorMsg: string): string =
4+
## Executes `cmd` at compile time and returns its text output (stdout + stderr).
5+
##
6+
## Raises an exception if the exit code is non-zero, using the given `errorMsg`.
7+
var exitCode = -1
8+
(result, exitCode) = gorgeEx(cmd)
9+
result.stripLineEnd()
10+
if exitCode != 0:
11+
echo result
12+
raise newException(OSError, errorMsg)
13+
14+
type
15+
PackagePaths = object
16+
cligen: string
17+
18+
proc init(T: typedesc[PackagePaths]): T =
19+
## Returns the absolute paths to Nimble packages that we patch.
20+
# Optimization: call `nimble path` only once.
21+
result = T()
22+
let output = block:
23+
var cmd = "nimble path"
24+
for fieldName, _ in result.fieldPairs:
25+
cmd.add " " & fieldName
26+
gorgeCheck(cmd, "failed to get path to packages")
27+
var i = 0
28+
for fieldVal in result.fields:
29+
i += output.parseUntil(fieldVal, {'\n'}, i) + 1
30+
31+
proc patch(dir, patchPath: string;
32+
files: varargs[tuple[relPath: string, patchedHash: int64]]) =
33+
## Checks that each file in `files` has the corresponding `patchedHash`, and
34+
## if not, applies the patch at `patchPath` to `dir`.
35+
##
36+
## Raises an exception if the patch could not be applied.
37+
# We want to support running `nimble build` before the package is installed,
38+
# so we can't `import foo` to check the package's behavior directly.
39+
# Instead, hash the files and then run `git apply` when necessary.
40+
# Use `std/hashes` to hash - note that we can't import `std/md5` or `std/sha1`
41+
# in a .nimble file.
42+
for (relPath, patchedHash) in files:
43+
if readFile(dir / relPath).hash().int64 != patchedHash:
44+
# Apply the patch.
45+
let cmd = "git -C " & dir & " apply --verbose " & patchPath
46+
discard gorgeCheck(cmd, "failed to apply patch")
47+
break
48+
49+
proc patchCligen(packagePaths: PackagePaths) =
50+
# Patch `cligen/parseopt3` so that "--foo --bar" is parsed as two long options,
51+
# even when `longNoVal` is both non-empty and lacks `foo`.
52+
const thisDir = currentSourcePath().parentDir()
53+
patch(
54+
packagePaths.cligen,
55+
thisDir / "parseopt3_allow_long_option_optional_value.patch",
56+
("cligen" / "parseopt3.nim", 1647921161'i64)
57+
)
58+
59+
proc ensureThatNimblePackagesArePatched* =
60+
let packagePaths = PackagePaths.init()
61+
patchCligen packagePaths

0 commit comments

Comments
 (0)