From 1d0f28e3219c0a65b59acb13e80edc986977cbfd Mon Sep 17 00:00:00 2001 From: Dominik Picheta Date: Fri, 23 Dec 2016 19:09:54 +0100 Subject: [PATCH] Preserve special versions in package installations. Ref #88. --- src/nimble.nim | 33 +++++---- src/nimblepkg/download.nim | 18 ++--- src/nimblepkg/packageinfo.nim | 13 +--- src/nimblepkg/packageparser.nim | 27 +++++-- src/nimblepkg/version.nim | 121 ++++++++++++++++++++------------ tests/tester.nim | 11 ++- 6 files changed, 139 insertions(+), 84 deletions(-) diff --git a/src/nimble.nim b/src/nimble.nim index 912cf5a..bdac95f 100644 --- a/src/nimble.nim +++ b/src/nimble.nim @@ -263,7 +263,7 @@ proc processDeps(pkginfo: PackageInfo, options: Options): seq[string] = result = @[] assert(not pkginfo.isMinimal, "processDeps needs pkginfo.requires") display("Verifying", - "dependencies for $1 v$2" % [pkginfo.name, pkginfo.version], + "dependencies for $1@$2" % [pkginfo.name, pkginfo.version], priority = HighPriority) let pkglist = getInstalledPkgs(options.getPkgsDir(), options) @@ -275,7 +275,7 @@ proc processDeps(pkginfo: PackageInfo, options: Options): seq[string] = let msg = "Unsatisfied dependency: " & dep.name & " (" & $dep.ver & ")" raise newException(NimbleError, msg) else: - let depDesc = "$1 ($2)" % [dep.name, $dep.ver] + let depDesc = "$1@$2" % [dep.name, $dep.ver] display("Checking", "for $1" % depDesc, priority = MediumPriority) var pkg: PackageInfo if not findPkg(pkglist, dep, pkg): @@ -390,7 +390,7 @@ proc vcsRevisionInDir(dir: string): string = except: discard -proc installFromDir(dir: string, latest: bool, options: Options, +proc installFromDir(dir: string, requestedVer: VersionRange, options: Options, url: string): tuple[paths: seq[string], pkg: PackageInfo] = ## Returns where package has been installed to, together with paths ## to the packages this package depends on. @@ -400,24 +400,29 @@ proc installFromDir(dir: string, latest: bool, options: Options, let realDir = pkgInfo.getRealDir() let binDir = options.getBinDir() let pkgsDir = options.getPkgsDir() - var deps_options = options - deps_options.depsOnly = false + var depsOptions = options + depsOptions.depsOnly = false + + # Overwrite the version if the requested version is "#head" or similar. + if requestedVer.kind == verSpecial: + pkgInfo.version = $requestedVer.spe # Dependencies need to be processed before the creation of the pkg dir. - result.paths = processDeps(pkginfo, deps_options) + result.paths = processDeps(pkginfo, depsOptions) if options.depsOnly: result.pkg = pkgInfo return result - display("Installing", "$1 v$2" % [pkginfo.name, pkginfo.version], + display("Installing", "$1 $2" % [pkginfo.name, pkginfo.version], priority = HighPriority) # Build before removing an existing package (if one exists). This way # if the build fails then the old package will still be installed. if pkgInfo.bin.len > 0: buildFromDir(pkgInfo, result.paths, true) - let versionStr = (if latest: "" else: '-' & pkgInfo.version) + let versionStr = '-' & pkgInfo.version + let pkgDestDir = pkgsDir / (pkgInfo.name & versionStr) if existsDir(pkgDestDir) and existsFile(pkgDestDir / "nimblemeta.json"): if not options.prompt(pkgInfo.name & versionStr & @@ -527,7 +532,7 @@ proc getNimbleTempDir(): string = proc downloadPkg(url: string, verRange: VersionRange, downMethod: DownloadMethod, - options: Options): (string, VersionRange) = + options: Options): (string, Version) = ## Downloads the repository as specified by ``url`` and ``verRange`` using ## the download method specified. ## @@ -580,7 +585,7 @@ proc install(packages: seq[PkgTuple], options: Options, doPrompt = true): tuple[paths: seq[string], pkg: PackageInfo] = if packages == @[]: - result = installFromDir(getCurrentDir(), false, options, "") + result = installFromDir(getCurrentDir(), newVRAny(), options, "") else: # If packages.json is not present ask the user if they want to download it. if needsRefresh(options): @@ -597,11 +602,11 @@ proc install(packages: seq[PkgTuple], let (downloadDir, downloadVersion) = downloadPkg(url, pv.ver, meth, options) try: - result = installFromDir(downloadDir, false, options, url) + result = installFromDir(downloadDir, pv.ver, options, url) except BuildFailed: # The package failed to build. # Check if we tried building a tagged version of the package. - let headVer = parseVersionRange("#" & getHeadName(meth)) + let headVer = getHeadName(meth) if pv.ver.kind != verSpecial and downloadVersion != headVer: # If we tried building a tagged version of the package then # ask the user whether they want to try building #head. @@ -610,8 +615,8 @@ proc install(packages: seq[PkgTuple], " like to try installing '$1@#head' (latest unstable)?") % [pv.name, $downloadVersion]) if promptResult: - - result = install(@[(pv.name, headVer)], options, doPrompt) + let toInstall = @[(pv.name, headVer.toVersionRange())] + result = install(toInstall, options, doPrompt) else: raise newException(BuildFailed, "Aborting installation due to build failure") diff --git a/src/nimblepkg/download.nim b/src/nimblepkg/download.nim index bb6f0e1..cedd648 100644 --- a/src/nimblepkg/download.nim +++ b/src/nimblepkg/download.nim @@ -116,12 +116,12 @@ proc getDownloadMethod*(meth: string): DownloadMethod = else: raise newException(NimbleError, "Invalid download method: " & meth) -proc getHeadName*(meth: DownloadMethod): string = +proc getHeadName*(meth: DownloadMethod): Version = ## Returns the name of the download method specific head. i.e. for git ## it's ``head`` for hg it's ``tip``. case meth - of DownloadMethod.git: "head" - of DownloadMethod.hg: "tip" + of DownloadMethod.git: newVersion("#head") + of DownloadMethod.hg: newVersion("#tip") proc checkUrlType*(url: string): DownloadMethod = ## Determines the download method based on the URL. @@ -136,7 +136,7 @@ proc isURL*(name: string): bool = name.startsWith(peg" @'://' ") proc doDownload*(url: string, downloadDir: string, verRange: VersionRange, - downMethod: DownloadMethod, options: Options): VersionRange = + downMethod: DownloadMethod, options: Options): Version = ## Downloads the repository specified by ``url`` using the specified download ## method. ## @@ -152,7 +152,7 @@ proc doDownload*(url: string, downloadDir: string, verRange: VersionRange, # https://github.com/nim-lang/nimble/issues/22 meth if $latest.ver != "": - result = parseVersionRange($latest.ver) + result = latest.ver else: # Result should already be set to #head here. assert(not result.isNil) @@ -170,20 +170,20 @@ proc doDownload*(url: string, downloadDir: string, verRange: VersionRange, removeDir(downloadDir) if verRange.kind == verSpecial: # We want a specific commit/branch/tag here. - if verRange.spe == newSpecial(getHeadName(downMethod)): + if verRange.spe == getHeadName(downMethod): doClone(downMethod, url, downloadDir) # Grab HEAD. else: # Grab the full repo. doClone(downMethod, url, downloadDir, tip = false) # Then perform a checkout operation to get the specified branch/commit. doCheckout(downMethod, downloadDir, $verRange.spe) - result = verRange + result = verRange.spe else: case downMethod of DownloadMethod.git: # For Git we have to query the repo remotely for its tags. This is # necessary as cloning with a --depth of 1 removes all tag info. - result = parseVersionRange("#head") + result = getHeadName(downMethod) let versions = getTagsListRemote(url, downMethod).getVersionList() if versions.len > 0: getLatestByTag: @@ -197,7 +197,7 @@ proc doDownload*(url: string, downloadDir: string, verRange: VersionRange, verifyClone() of DownloadMethod.hg: doClone(downMethod, url, downloadDir) - result = parseVersionRange("#tip") + result = getHeadName(downMethod) let versions = getTagsList(downloadDir, downMethod).getVersionList() if versions.len > 0: diff --git a/src/nimblepkg/packageinfo.nim b/src/nimblepkg/packageinfo.nim index d598570..f278e63 100644 --- a/src/nimblepkg/packageinfo.nim +++ b/src/nimblepkg/packageinfo.nim @@ -4,7 +4,7 @@ import parsecfg, json, streams, strutils, parseutils, os, sets, tables import version, tools, common, options, cli type - Package* = object + Package* = object ## Definition of package from packages.json. # Required fields in a package. name*: string url*: string # Download location. @@ -57,7 +57,6 @@ proc getNameVersion*(pkgpath: string): tuple[name, version: string] = ## ## Also works for file paths like: ## ``/home/user/.nimble/pkgs/package-0.1/package.nimble`` - if pkgPath.splitFile.ext == ".nimble" or pkgPath.splitFile.ext == ".babel": return getNameVersion(pkgPath.splitPath.head) @@ -288,14 +287,8 @@ when isMainModule: ("package-a", "0.1") doAssert getNameVersion("/home/user/.nimble/libs/package-a-0.1/package.nimble") == ("package-a", "0.1") - - validatePackageName("foo_bar") - validatePackageName("f_oo_b_a_r") - try: - validatePackageName("foo__bar") - assert false - except NimbleError: - assert true + doAssert getNameVersion("/home/user/.nimble/libs/package-#head") == + ("package", "#head") doAssert toValidPackageName("foo__bar") == "foo_bar" doAssert toValidPackageName("jhbasdh!£$@%#^_&*_()qwe") == "jhbasdh_qwe" diff --git a/src/nimblepkg/packageparser.nim b/src/nimblepkg/packageparser.nim index 1d8c1f9..819b6c8 100644 --- a/src/nimblepkg/packageparser.nim +++ b/src/nimblepkg/packageparser.nim @@ -193,6 +193,7 @@ proc readPackageInfo*(nf: NimbleFile, options: Options, return options.pkgInfoCache[nf] result = initPackageInfo(nf) + let minimalInfo = getNameVersion(nf) validatePackageName(nf.splitFile.name) @@ -208,9 +209,8 @@ proc readPackageInfo*(nf: NimbleFile, options: Options, if not success: if onlyMinimalInfo: - let tmp = getNameVersion(nf) - result.name = tmp.name - result.version = tmp.version + result.name = minimalInfo.name + result.version = minimalInfo.version result.isNimScript = true result.isMinimal = true else: @@ -226,6 +226,14 @@ proc readPackageInfo*(nf: NimbleFile, options: Options, raise newException(NimbleError, msg) validatePackageInfo(result, nf) + + # The package directory name may include a "special" version + # (example #head). If so, it is given higher priority and therefore + # overwrites the .nimble file's version. + let version = parseVersionRange(minimalInfo.version) + if version.kind == verSpecial: + result.version = minimalInfo.version + if not result.isMinimal: options.pkgInfoCache[nf] = result @@ -240,7 +248,7 @@ proc getInstalledPkgs*(libsDir: string, options: Options): ## ## ``libsDir`` is in most cases: ~/.nimble/pkgs/ const - readErrorMsg = "Installed package $1 v$2 is outdated or corrupt." + readErrorMsg = "Installed package '$1@$2' is outdated or corrupt." validationErrorMsg = readErrorMsg & "\nPackage did not pass validation: $3" hintMsg = "The corrupted package will need to be removed manually. To fix" & " this error message, remove $1." @@ -278,3 +286,14 @@ proc getInstalledPkgs*(libsDir: string, options: Options): proc isNimScript*(nf: string, options: Options): bool = result = readPackageInfo(nf, options).isNimScript + +when isMainModule: + validatePackageName("foo_bar") + validatePackageName("f_oo_b_a_r") + try: + validatePackageName("foo__bar") + assert false + except NimbleError: + assert true + + echo("Everything passed!") diff --git a/src/nimblepkg/version.nim b/src/nimblepkg/version.nim index a683c2e..080c469 100644 --- a/src/nimblepkg/version.nim +++ b/src/nimblepkg/version.nim @@ -5,7 +5,6 @@ import strutils, tables, hashes, parseutils type Version* = distinct string - Special* = distinct string VersionRangeEnum* = enum verLater, # > V @@ -23,7 +22,7 @@ type of verLater, verEarlier, verEqLater, verEqEarlier, verEq: ver*: Version of verSpecial: - spe*: Special + spe*: Version of verIntersect: verILeft, verIRight: VersionRange of verAny: @@ -36,18 +35,25 @@ type NimbleError* = object of Exception hint*: string -proc newVersion*(ver: string): Version = return Version(ver) -proc newSpecial*(spe: string): Special = return Special(spe) +proc newVersion*(ver: string): Version = + doAssert(ver[0] in {'#', '\0'} + Digits) + return Version(ver) proc `$`*(ver: Version): string {.borrow.} proc hash*(ver: Version): Hash {.borrow.} -proc `$`*(ver: Special): string {.borrow.} +proc isNil*(ver: Version): bool {.borrow.} -proc hash*(ver: Special): Hash {.borrow.} +proc isSpecial*(ver: Version): bool = + return ($ver)[0] == '#' proc `<`*(ver: Version, ver2: Version): bool = + # Handling for special versions such as "#head" or "#branch". + if ver.isSpecial or ver2.isSpecial: + return false + + # Handling for normal versions such as "0.1.0" or "1.0". var sVer = string(ver).split('.') var sVer2 = string(ver2).split('.') for i in 0..max(sVer.len, sVer2.len)-1: @@ -65,6 +71,9 @@ proc `<`*(ver: Version, ver2: Version): bool = return false proc `==`*(ver: Version, ver2: Version): bool = + if ver.isSpecial or ver2.isSpecial: + return ($ver).toLowerAscii() == ($ver2).toLowerAscii() + var sVer = string(ver).split('.') var sVer2 = string(ver2).split('.') for i in 0..max(sVer.len, sVer2.len)-1: @@ -79,9 +88,6 @@ proc `==`*(ver: Version, ver2: Version): bool = else: return false -proc `==`*(spe: Special, spe2: Special): bool = - return ($spe).toLowerAscii() == ($spe2).toLowerAscii() - proc `<=`*(ver: Version, ver2: Version): bool = return (ver == ver2) or (ver < ver2) @@ -97,39 +103,36 @@ proc `==`*(range1: VersionRange, range2: VersionRange): bool = of verAny: true proc withinRange*(ver: Version, ran: VersionRange): bool = - case ran.kind - of verLater: - return ver > ran.ver - of verEarlier: - return ver < ran.ver - of verEqLater: - return ver >= ran.ver - of verEqEarlier: - return ver <= ran.ver - of verEq: - return ver == ran.ver - of verSpecial: - return false - of verIntersect: - return withinRange(ver, ran.verILeft) and withinRange(ver, ran.verIRight) - of verAny: - return true - -proc withinRange*(spe: Special, ran: VersionRange): bool = - case ran.kind - of verLater, verEarlier, verEqLater, verEqEarlier, verEq, verIntersect: - return false - of verSpecial: - return spe == ran.spe - of verAny: - return true + if ver.isSpecial: + case ran.kind + of verLater, verEarlier, verEqLater, verEqEarlier, verEq, verIntersect: + return false + of verSpecial: + return ver == ran.spe + of verAny: + return true + else: + case ran.kind + of verLater: + return ver > ran.ver + of verEarlier: + return ver < ran.ver + of verEqLater: + return ver >= ran.ver + of verEqEarlier: + return ver <= ran.ver + of verEq: + return ver == ran.ver + of verSpecial: + return false + of verIntersect: + return withinRange(ver, ran.verILeft) and withinRange(ver, ran.verIRight) + of verAny: + return true proc contains*(ran: VersionRange, ver: Version): bool = return withinRange(ver, ran) -proc contains*(ran: VersionRange, spe: Special): bool = - return withinRange(spe, ran) - proc makeRange*(version: string, op: string): VersionRange = new(result) if version == "": @@ -153,9 +156,13 @@ proc makeRange*(version: string, op: string): VersionRange = proc parseVersionRange*(s: string): VersionRange = # >= 1.5 & <= 1.8 new(result) + if s.len == 0: + result.kind = verAny + return + if s[0] == '#': result.kind = verSpecial - result.spe = s[1 .. s.len-1].Special + result.spe = s.Version return var i = 0 @@ -200,6 +207,15 @@ proc parseVersionRange*(s: string): VersionRange = "Unexpected char in version range: " & s[i]) inc(i) +proc toVersionRange*(ver: Version): VersionRange = + ## Converts a version to either a verEq or verSpecial VersionRange. + new(result) + if ver.isSpecial: + result.kind = verSpecial + result.spe = ver + else: + result.kind = verEq + result.ver = ver proc parseRequires*(req: string): PkgTuple = try: @@ -231,7 +247,7 @@ proc `$`*(verRange: VersionRange): string = of verEq: result = "" of verSpecial: - return "#" & $verRange.spe + return $verRange.spe of verIntersect: return $verRange.verILeft & " & " & $verRange.verIRight of verAny: @@ -312,13 +328,26 @@ when isMainModule: #doAssert newVersion("0.1-rc1") < newVersion("0.1") # Special tests - doAssert newSpecial("ab26sgdt362") != newSpecial("ab26saggdt362") - doAssert newSpecial("ab26saggdt362") == newSpecial("ab26saggdt362") - doAssert newSpecial("head") == newSpecial("HEAD") - doAssert newSpecial("head") == newSpecial("head") + doAssert newVersion("#ab26sgdt362") != newVersion("#qwersaggdt362") + doAssert newVersion("#ab26saggdt362") == newVersion("#ab26saggdt362") + doAssert newVersion("#head") == newVersion("#HEAD") + doAssert newVersion("#head") == newVersion("#head") var sp = parseVersionRange("#ab26sgdt362") - doAssert newSpecial("ab26sgdt362") in sp - doAssert newSpecial("ab26saggdt362") notin sp + doAssert newVersion("#ab26sgdt362") in sp + doAssert newVersion("#ab26saggdt362") notin sp + + doAssert newVersion("#head") in parseVersionRange("#head") + + # TODO: It may be worth changing this in the future, although we can't be + # certain that #head is in fact newer than v0.1.0. + doAssert(not(newVersion("#head") > newVersion("0.1.0"))) + + # An empty version range should give verAny + doAssert parseVersionRange("").kind == verAny + + # toVersionRange tests + doAssert toVersionRange(newVersion("#head")).kind == verSpecial + doAssert toVersionRange(newVersion("0.2.0")).kind == verEq echo("Everything works!") diff --git a/tests/tester.nim b/tests/tester.nim index 69938dd..13c1040 100644 --- a/tests/tester.nim +++ b/tests/tester.nim @@ -2,6 +2,10 @@ # BSD License. Look at license.txt for more info. import osproc, streams, unittest, strutils, os, sequtils, future +# TODO: Each test should start off with a clean slate. Currently installed +# packages are shared between each test which causes a multitude of issues +# and is really fragile. + var rootDir = getCurrentDir().parentDir() var nimblePath = rootDir / "src" / "nimble" & ExeExt var installDir = rootDir / "tests" / "nimbleDir" @@ -41,6 +45,11 @@ test "issue 129 (installing commit hash)": let arguments = @["install", "-y", "https://github.com/nimble-test/packagea.git@#1f9cb289c89"] check execNimble(arguments).exitCode == QuitSuccess + # Verify that it was installed correctly. + check dirExists(installDir / "pkgs" / "packagea-#1f9cb289c89") + # Remove it so that it doesn't interfere with the uninstall tests. + check execNimble("uninstall", "-y", "packagea@#1f9cb289c89").exitCode == + QuitSuccess test "issue 113 (uninstallation problems)": cd "issue113/c": @@ -247,6 +256,6 @@ test "can uninstall": check execNimble("uninstall", "-y", "PackageA@0.2", "issue27b").exitCode == QuitSuccess - check(not dirExists(getHomeDir() / ".nimble" / "pkgs" / "PackageA-0.2.0")) + check(not dirExists(installDir / "pkgs" / "PackageA-0.2.0")) check execNimble("uninstall", "-y", "nimscript").exitCode == QuitSuccess