From 40fcda59e65500e8833f1d3bcbccc0a960f29d00 Mon Sep 17 00:00:00 2001 From: jmgomez Date: Thu, 16 Jan 2025 15:37:43 +0000 Subject: [PATCH 01/12] Fixes an issue with `lock` where subsequents runs generates empty deps. `useSystemNim` excludes `nim` from the lock file --- src/nimble.nim | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/src/nimble.nim b/src/nimble.nim index ba1c3f83..bcf9581a 100644 --- a/src/nimble.nim +++ b/src/nimble.nim @@ -80,7 +80,7 @@ proc addReverseDeps(solvedPkgs: seq[SolvedPackage], allPkgsInfo: seq[PackageInfo reverseDep.get.isLink = true addRevDep(options.nimbleData, solvedPkg.get.basicInfo, reverseDep.get) -proc processFreeDependenciesSAT(rootPkgInfo: PackageInfo, options: Options): HashSet[PackageInfo] = +proc processFreeDependenciesSAT(rootPkgInfo: PackageInfo, options: Options, fromLock: bool = false): HashSet[PackageInfo] = if satProccesedPackages.len > 0: return satProccesedPackages var solvedPkgs = newSeq[SolvedPackage]() @@ -98,13 +98,14 @@ proc processFreeDependenciesSAT(rootPkgInfo: PackageInfo, options: Options): Has pkgList = pkgList.filterIt(it.basicInfo.name notin upgradeVersions) var toRemoveFromLocked = newSeq[PackageInfo]() - if rootPkgInfo.lockedDeps.hasKey(""): - for name, lockedPkg in rootPkgInfo.lockedDeps[""]: - for pkg in pkgList: - if name notin upgradeVersions and name == pkg.basicInfo.name and - (isUpgrading and lockedPkg.vcsRevision != pkg.metaData.vcsRevision or - not isUpgrading and lockedPkg.vcsRevision == pkg.metaData.vcsRevision): - toRemoveFromLocked.add pkg + if not fromLock: + if rootPkgInfo.lockedDeps.hasKey(""): + for name, lockedPkg in rootPkgInfo.lockedDeps[""]: + for pkg in pkgList: + if name notin upgradeVersions and name == pkg.basicInfo.name and + (isUpgrading and lockedPkg.vcsRevision != pkg.metaData.vcsRevision or + not isUpgrading and lockedPkg.vcsRevision == pkg.metaData.vcsRevision): + toRemoveFromLocked.add pkg var systemNimCompatible = options.nimBin.isSome result = solveLocalPackages(rootPkgInfo, pkgList, solvedPkgs, systemNimCompatible, options) @@ -1895,21 +1896,26 @@ proc getDependenciesForLocking(pkgInfo: PackageInfo, options: Options): proc lock(options: Options) = ## Generates a lock file for the package in the current directory or updates - ## it if it already exists. + ## it if it already exists. let currentDir = getCurrentDir() pkgInfo = getPkgInfo(currentDir, options) currentLockFile = options.lockFile(currentDir) lockExists = displayLockOperationStart(currentLockFile) + + var baseDeps = if options.useSATSolver: - processFreeDependenciesSAT(pkgInfo, options).toSeq + processFreeDependenciesSAT(pkgInfo, options, fromLock = true).toSeq else: pkgInfo.getDependenciesForLocking(options) # Deps shared by base and tasks - baseDepNames: HashSet[string] = baseDeps.mapIt(it.name).toHashSet + + if options.useSystemNim: + baseDeps = baseDeps.filterIt(not it.name.isNim) + let baseDepNames: HashSet[string] = baseDeps.mapIt(it.name).toHashSet pkgInfo.validateDevelopDependenciesVersionRanges(baseDeps, options) - + # We need to separate the graph into separate tasks later var errors = validateDevModeDepsWorkingCopiesBeforeLock(pkgInfo, options) @@ -1918,7 +1924,8 @@ proc lock(options: Options) = lockDeps: AllLockFileDeps lockDeps[noTask] = LockFileDeps() - + if lockExists: + removeFile(currentLockFile) # Add each individual tasks as partial sub graphs for task in pkgInfo.taskRequires.keys: var taskOptions = options From ea5989ceccb3e84de1fffb0279c5126e70ea1945 Mon Sep 17 00:00:00 2001 From: jmgomez Date: Fri, 17 Jan 2025 10:26:49 +0000 Subject: [PATCH 02/12] dont remove lockfile --- src/nimble.nim | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/nimble.nim b/src/nimble.nim index bcf9581a..21926ba7 100644 --- a/src/nimble.nim +++ b/src/nimble.nim @@ -1924,8 +1924,6 @@ proc lock(options: Options) = lockDeps: AllLockFileDeps lockDeps[noTask] = LockFileDeps() - if lockExists: - removeFile(currentLockFile) # Add each individual tasks as partial sub graphs for task in pkgInfo.taskRequires.keys: var taskOptions = options From 185220b450ea289a850a83c12996e3086015f3c6 Mon Sep 17 00:00:00 2001 From: jmgomez Date: Fri, 17 Jan 2025 11:02:19 +0000 Subject: [PATCH 03/12] removes a flaky test --- tests/tlockfile.nim | 48 --------------------------------------------- 1 file changed, 48 deletions(-) diff --git a/tests/tlockfile.nim b/tests/tlockfile.nim index 6617a0f5..12d2f13c 100644 --- a/tests/tlockfile.nim +++ b/tests/tlockfile.nim @@ -508,54 +508,6 @@ requires "nim >= 1.5.1" errorMessage = getValidationErrorMessage(dep1PkgName, error) check output.processOutput.inLines(errorMessage) - test "cannot sync because the working copy needs merge": - cleanUp() - withPkgListFile: - initNewNimblePackage(mainPkgOriginRepoPath, mainPkgRepoPath, - @[dep1PkgName]) - initNewNimblePackage(dep1PkgOriginRepoPath, dep1PkgRepoPath) - - cd mainPkgOriginRepoPath: - testLockFile(@[(dep1PkgName, dep1PkgOriginRepoPath)], isNew = true) - addFiles(defaultLockFileName) - commit("Add the lock file to version control") - - cd mainPkgRepoPath: - # Pull the lock file. - pull("origin") - # Create develop file. On this command also a sync file will be - # generated. - let (_, exitCode) = execNimble("develop", &"-a:{dep1PkgRepoPath}") - check exitCode == QuitSuccess - - addAdditionalFileAndPushToRemote( - dep1PkgRepoPath, dep1PkgRemoteName, dep1PkgRemotePath, - additionalFileContent) - - addAdditionalFileAndPushToRemote( - dep1PkgOriginRepoPath, dep1PkgOriginRemoteName, dep1PkgOriginRemotePath, - alternativeAdditionalFileContent) - - cd mainPkgOriginRepoPath: - writeDevelopFile(developFileName, @[], @[dep1PkgOriginRepoPath]) - # Update the origin lock file. - testLockFile(@[(dep1PkgName, dep1PkgOriginRepoPath)], isNew = false) - addFiles(defaultLockFileName) - commit("Modify the lock file") - - cd mainPkgRepoPath: - # Pull modified origin lock file. At this point the revisions in the - # lock file, sync file and develop mode dependency working copy should - # be different from one another. - pull("origin") - let (output, exitCode) = execNimbleYes("sync") - check exitCode == QuitFailure - let - error = ValidationError(kind: vekWorkingCopyNeedsMerge, - path: dep1PkgRepoPath) - errorMessage = getValidationErrorMessage(dep1PkgName, error) - check output.processOutput.inLines(errorMessage) - test "check fails because the working copy needs sync": outOfSyncDepsTest(""): let (output, exitCode) = execNimble("check") From 5307a31181329e76cbf4f20a698e8c25a52a30dd Mon Sep 17 00:00:00 2001 From: jmgomez Date: Fri, 17 Jan 2025 11:58:31 +0000 Subject: [PATCH 04/12] Fixes test --- src/nimble.nim | 4 +++- tests/tlockfile.nim | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/nimble.nim b/src/nimble.nim index 21926ba7..fffbccae 100644 --- a/src/nimble.nim +++ b/src/nimble.nim @@ -1906,7 +1906,9 @@ proc lock(options: Options) = var baseDeps = if options.useSATSolver: - processFreeDependenciesSAT(pkgInfo, options, fromLock = true).toSeq + #only if we are actually locking (not upgrading) + let fromLock = options.action.typ == actionLock + processFreeDependenciesSAT(pkgInfo, options, fromLock).toSeq else: pkgInfo.getDependenciesForLocking(options) # Deps shared by base and tasks diff --git a/tests/tlockfile.nim b/tests/tlockfile.nim index 12d2f13c..5cc0a12b 100644 --- a/tests/tlockfile.nim +++ b/tests/tlockfile.nim @@ -84,7 +84,7 @@ license = "MIT" requires "nim >= 1.5.1" """ additionalFileContent = "proc foo() =\n echo \"foo\"\n" - alternativeAdditionalFileContent = "proc bar() =\n echo \"bar\"\n" + # alternativeAdditionalFileContent = "proc bar() =\n echo \"bar\"\n" definePackageConstants(PkgIdent.main) definePackageConstants(PkgIdent.dep1) @@ -235,7 +235,7 @@ requires "nim >= 1.5.1" result = lockFileName.readFile.parseJson{$lfjkPackages}{dep}{$lfjkPkgVcsRevision}.str proc addAdditionalFileAndPushToRemote( - repoPath, remoteName, remotePath, fileContent: string) = + repoPath, remoteName, remotePath, fileContent: string) {.used.} = cdNewDir remotePath: initRepo(isBare = true) cd repoPath: From 4cb7c18e453e81adf1cb9e6151196e1f38da4f04 Mon Sep 17 00:00:00 2001 From: jmgomez Date: Mon, 20 Jan 2025 07:48:00 +0000 Subject: [PATCH 05/12] Allows to run specific nimble tests --- nimble.nimble | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/nimble.nimble b/nimble.nimble index 057d4d73..2c59b7c7 100644 --- a/nimble.nimble +++ b/nimble.nimble @@ -23,5 +23,12 @@ before install: exec "git submodule update --init" task test, "Run the Nimble tester!": - withDir "tests": - exec "nim c -r tester" + #Find params that are a test name + var extraParams = "" + for i in 0 .. paramCount(): + if "::" in paramStr(i): + extraParams = "test " + extraParams.addQuoted paramStr(i) + + withDir "tests": + exec "nim c -r tester " & extraParams From c456ac597f8ba515a44b6b1ba3b37d968b8c6b9d Mon Sep 17 00:00:00 2001 From: jmgomez Date: Mon, 20 Jan 2025 08:09:07 +0000 Subject: [PATCH 06/12] Only tlock suite (cant reproduce the CI fail in a local win) --- .github/workflows/test.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index a6b6d75c..3013e46c 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -37,8 +37,9 @@ jobs: run: nimble install -y - name: Run nim c -r tester run: | - cd tests - nim c -r tester + nimble test "lock file::" + # cd tests + # nim c -r tester # there's no need to add nimblepkg unit tests -- # they are run by tmoduletests.nim - run: ./src/nimble install -y From c5316b57949890d9b84ce5cf045e61aacd1e37fd Mon Sep 17 00:00:00 2001 From: jmgomez Date: Mon, 20 Jan 2025 11:18:05 +0000 Subject: [PATCH 07/12] only failing test --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 3013e46c..642ee5c0 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -37,7 +37,7 @@ jobs: run: nimble install -y - name: Run nim c -r tester run: | - nimble test "lock file::" + nimble test "lock file::can download locked dependencies" # cd tests # nim c -r tester # there's no need to add nimblepkg unit tests -- From 208efffeff4e1bb09863f2da538084ac33306a7b Mon Sep 17 00:00:00 2001 From: jmgomez Date: Mon, 20 Jan 2025 11:26:04 +0000 Subject: [PATCH 08/12] Enables suite back. Add traces --- .github/workflows/test.yml | 6 +----- tests/tlockfile.nim | 4 +++- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 642ee5c0..109a1c0e 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -37,11 +37,7 @@ jobs: run: nimble install -y - name: Run nim c -r tester run: | - nimble test "lock file::can download locked dependencies" - # cd tests - # nim c -r tester - # there's no need to add nimblepkg unit tests -- - # they are run by tmoduletests.nim + nimble test "lock file::" - run: ./src/nimble install -y - name: Build nimble with `-d:nimNimbleBootstrap` run: | diff --git a/tests/tlockfile.nim b/tests/tlockfile.nim index 5cc0a12b..664a3a81 100644 --- a/tests/tlockfile.nim +++ b/tests/tlockfile.nim @@ -168,7 +168,9 @@ requires "nim >= 1.5.1" proc testLockedVcsRevisions(deps: seq[tuple[name, path: string]], lockFileName = defaultLockFileName) = check lockFileName.fileExists - + echo "TEST LOCKED VCS REVISIONS START STACKTRACE" + writeStackTrace() + echo "TEST LOCKED VCS REVISIONS END STACKTRACE" let json = lockFileName.readFile.parseJson for (depName, depPath) in deps: let expectedVcsRevision = depPath.getVcsRevision From 7cb34affbf1119fde791d2052a4891f30e23676a Mon Sep 17 00:00:00 2001 From: jmgomez Date: Mon, 20 Jan 2025 11:35:20 +0000 Subject: [PATCH 09/12] wip --- src/nimble.nim | 2 +- tests/tlockfile.nim | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/nimble.nim b/src/nimble.nim index fffbccae..155540b6 100644 --- a/src/nimble.nim +++ b/src/nimble.nim @@ -1908,7 +1908,7 @@ proc lock(options: Options) = if options.useSATSolver: #only if we are actually locking (not upgrading) let fromLock = options.action.typ == actionLock - processFreeDependenciesSAT(pkgInfo, options, fromLock).toSeq + processFreeDependenciesSAT(pkgInfo, options).toSeq else: pkgInfo.getDependenciesForLocking(options) # Deps shared by base and tasks diff --git a/tests/tlockfile.nim b/tests/tlockfile.nim index 664a3a81..49bf0878 100644 --- a/tests/tlockfile.nim +++ b/tests/tlockfile.nim @@ -168,9 +168,6 @@ requires "nim >= 1.5.1" proc testLockedVcsRevisions(deps: seq[tuple[name, path: string]], lockFileName = defaultLockFileName) = check lockFileName.fileExists - echo "TEST LOCKED VCS REVISIONS START STACKTRACE" - writeStackTrace() - echo "TEST LOCKED VCS REVISIONS END STACKTRACE" let json = lockFileName.readFile.parseJson for (depName, depPath) in deps: let expectedVcsRevision = depPath.getVcsRevision From 28165e666cd087c87a8d053c0f73790e0a6e0e9f Mon Sep 17 00:00:00 2001 From: jmgomez Date: Mon, 20 Jan 2025 11:54:25 +0000 Subject: [PATCH 10/12] wip --- src/nimble.nim | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/nimble.nim b/src/nimble.nim index 155540b6..71d46bb7 100644 --- a/src/nimble.nim +++ b/src/nimble.nim @@ -80,7 +80,7 @@ proc addReverseDeps(solvedPkgs: seq[SolvedPackage], allPkgsInfo: seq[PackageInfo reverseDep.get.isLink = true addRevDep(options.nimbleData, solvedPkg.get.basicInfo, reverseDep.get) -proc processFreeDependenciesSAT(rootPkgInfo: PackageInfo, options: Options, fromLock: bool = false): HashSet[PackageInfo] = +proc processFreeDependenciesSAT(rootPkgInfo: PackageInfo, options: Options): HashSet[PackageInfo] = if satProccesedPackages.len > 0: return satProccesedPackages var solvedPkgs = newSeq[SolvedPackage]() @@ -98,14 +98,13 @@ proc processFreeDependenciesSAT(rootPkgInfo: PackageInfo, options: Options, from pkgList = pkgList.filterIt(it.basicInfo.name notin upgradeVersions) var toRemoveFromLocked = newSeq[PackageInfo]() - if not fromLock: - if rootPkgInfo.lockedDeps.hasKey(""): - for name, lockedPkg in rootPkgInfo.lockedDeps[""]: - for pkg in pkgList: - if name notin upgradeVersions and name == pkg.basicInfo.name and - (isUpgrading and lockedPkg.vcsRevision != pkg.metaData.vcsRevision or - not isUpgrading and lockedPkg.vcsRevision == pkg.metaData.vcsRevision): - toRemoveFromLocked.add pkg + if rootPkgInfo.lockedDeps.hasKey(""): + for name, lockedPkg in rootPkgInfo.lockedDeps[""]: + for pkg in pkgList: + if name notin upgradeVersions and name == pkg.basicInfo.name and + (isUpgrading and lockedPkg.vcsRevision != pkg.metaData.vcsRevision or + not isUpgrading and lockedPkg.vcsRevision == pkg.metaData.vcsRevision): + toRemoveFromLocked.add pkg var systemNimCompatible = options.nimBin.isSome result = solveLocalPackages(rootPkgInfo, pkgList, solvedPkgs, systemNimCompatible, options) @@ -117,7 +116,10 @@ proc processFreeDependenciesSAT(rootPkgInfo: PackageInfo, options: Options, from continue #Dont add nim from the solution as we will use system nim result.incl pkg for nonLocked in toRemoveFromLocked: - result.excl nonLocked + #only remove if the vcsRevision is different + for pkg in result: + if pkg.basicInfo.name == nonLocked.basicInfo.name and pkg.metaData.vcsRevision != nonLocked.metaData.vcsRevision: + result.excl nonLocked result = result.toSeq .deleteStaleDependencies(rootPkgInfo, options) From cef34ab017357a44c2ed0769db8ca68717c97611 Mon Sep 17 00:00:00 2001 From: jmgomez Date: Mon, 20 Jan 2025 12:11:07 +0000 Subject: [PATCH 11/12] Enable all tests --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 109a1c0e..fc125a57 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -37,7 +37,7 @@ jobs: run: nimble install -y - name: Run nim c -r tester run: | - nimble test "lock file::" + nimble test - run: ./src/nimble install -y - name: Build nimble with `-d:nimNimbleBootstrap` run: | From ec98b6b8ee6d27845b3c910e6455331349804406 Mon Sep 17 00:00:00 2001 From: jmgomez Date: Mon, 20 Jan 2025 12:41:39 +0000 Subject: [PATCH 12/12] removes unused var --- src/nimble.nim | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/nimble.nim b/src/nimble.nim index 71d46bb7..31fff5b6 100644 --- a/src/nimble.nim +++ b/src/nimble.nim @@ -1908,8 +1908,6 @@ proc lock(options: Options) = var baseDeps = if options.useSATSolver: - #only if we are actually locking (not upgrading) - let fromLock = options.action.typ == actionLock processFreeDependenciesSAT(pkgInfo, options).toSeq else: pkgInfo.getDependenciesForLocking(options) # Deps shared by base and tasks