Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Step To Verify Yarn and NPM Packages Are In Sync #617

Merged
merged 17 commits into from
Oct 10, 2022
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions azure-pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,23 @@ stages:
npm install tslint --verbose
npm run lint
displayName: Run Lint
##### Verify NPM and Yarn are in sync #####
- job: SyncPackageManagers
displayName: 'Verify NPM & Yarn In-Sync [Local Copy of Target Branch Must Be Up to Date]'
pool:
vmImage: 'windows-latest'
steps:
- task: UsePythonVersion@0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, out of curiosity why use Python? Familiarity I assume?

Copy link
Member Author

@nagilson nagilson Oct 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, familiarity + speed of development time. Writing this in bash sounds... not fun. C# is an option though it's also not really in the repository much so either way it's an introduced new onboarding cost. My hope is someday VSCE will support these parts of npm and this script can be eradicated, or AzDo will support yarn and we can eliminate NPM, but since their issues for NPM have been open for a few years now, I don't expect it to happen anytime soon

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes total sense!

@dbaeumer is there someone we can chat with to better understand VSCE expectations / details around yarn / npm integrations?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for linking, I did reach out to Joao. Here are the relevant issues in VSCE's repo microsoft/vscode-vsce#308
microsoft/vscode-vsce#580
microsoft/vscode-vsce#300
microsoft/vscode-vsce#381

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@NTaylorMullen the person to talk to is @joaomoreno

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My hope is someday VSCE will support these parts of npm

What specific features are you referring to?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What specific features?

Thanks for following up (offline as well a while back.) All of the above are relevant but here are the main 2: microsoft/vscode-vsce#308 and microsoft/vscode-vsce#580

inputs:
versionSpec: '3.x'
addToPath: true
architecture: 'x64'
- task: PythonScript@0
inputs:
scriptSource: 'filePath'
scriptPath: 'dependency-verifier.py'
arguments: '$(System.PullRequest.TargetBranch)'
failOnStderr: true
##### Package and Publish #####
- job: Package
displayName: 'Package and Publish'
Expand Down
85 changes: 85 additions & 0 deletions dependency-verifier.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
import sys
import subprocess
import os
from pathlib import Path


def main():
"""Check if the dependency updates in package-lock are also updated in yarn.locks"""
targetBranch = sys.argv[1] # Script is called with PR Target Branch Name, Fulfilled by AzDo
subprocess.getoutput(f"git fetch --all")
subprocess.getoutput(f"git pull origin {targetBranch}")
VerifyDependencies(targetBranch)
sys.exit(0)

def VerifyDependencies(targetBranch):
"""Enumerate through all changed files to check diffs."""
# origin/ requires origin/ to be up to date.
changedFiles = [Path(path) for path in subprocess.getoutput(f"git diff --name-only origin/{targetBranch}..").splitlines()]
npmLockFile = "package-lock.json"

for file in changedFiles:
fileName = os.path.basename(os.path.realpath(file))
if fileName == npmLockFile:
NpmChangesMirrorYarnChanges(changedFiles, file, targetBranch)

def GetNpmDependencyUpdates(packageLockDiffLines):
"""Returns a dictionary of [dependency -> [] (can be changed to version in later implementations)] changes found in diff string of package-lock.json"""
# Assumes dependency line starts with "node_modules/DEPENDENCYNAME". Version may or may not come after
dependencies = {}
for line in packageLockDiffLines:
line = line.strip()
line = line.lstrip("\t")
if line.startswith('"node_modules/'):
dependencies[line.split('"node_modules/', 1)[1].split('"', 1)[0]] = [] # will be "node_modules/dep further" content, need to cull
return dependencies

def GetYarnDependencyUpdates(yarnLockDiffLines):
"""Returns a dictionary of [dependency -> [] (can be changed to version in later implementations)] changes found in diff string of yarn.lock"""
# Assumes dependency line starts with DEPEDENCY@Version without whitespace
dependencies = {}
for line in yarnLockDiffLines:
if line == line.lstrip() and "@" in line:
depsAtVers = line.lstrip('"').split(",") # multiple dependencies are possible with diff versions, sep by ,
for dependencyAtVers in depsAtVers:
dep = dependencyAtVers.rsplit("@", 1)[0]
vers = dependencyAtVers.rsplit("@", 1)[1]
dependencies[dep] = [] # Could add version here later. (TODO) that will probably not happen
return dependencies

def GetUnmatchedDiffs(yarnDiff, npmDiff):
"""Returns [] if dependency updates are reflected in both diffs, elsewise the dependencies out of sync."""
# v Remove + or - from diff and additional git diff context lines
yarnDeps = GetYarnDependencyUpdates([line[1:] for line in yarnDiff.splitlines() if line.startswith("+") or line.startswith("-")])
npmDeps = GetNpmDependencyUpdates([line[1:] for line in npmDiff.splitlines() if line.startswith("+") or line.startswith("-")])
outOfSyncDependencies = []
for dep in npmDeps:
if dep in yarnDeps and yarnDeps[dep] == npmDeps[dep]: # version changes match
continue
else:
outOfSyncDependencies.append(dep)
return outOfSyncDependencies

def NpmChangesMirrorYarnChanges(changedFiles, packageLockPath, targetBranch):
"""Returns successfully if yarn.lock matches packagelock changes, if not, throws exit code"""
yarnLockFile = "yarn.lock"
yarnLockPath = Path(os.path.join(os.path.dirname(packageLockPath), yarnLockFile))
outOfDateYarnLocks = []

if yarnLockPath in changedFiles:
yarnDiff = subprocess.getoutput(f"git diff origin/{targetBranch}.. -- {str(yarnLockPath)}")
npmDiff = subprocess.getoutput(f"git diff origin/{targetBranch}.. -- {packageLockPath}")
diffSetComplement = GetUnmatchedDiffs(yarnDiff, npmDiff)
if diffSetComplement == []:
pass
else:
outOfDateYarnLocks.append((str(yarnLockPath), diffSetComplement))
else:
outOfDateYarnLocks.append(yarnLockPath)
if(outOfDateYarnLocks != []):
sys.exit(f"The yarn.lock and package-lock appear to be out of sync with the changes made after {targetBranch}. Update by doing yarn import or yarn add dep@package-lock-version for {outOfDateYarnLocks}. For sub-dependencies, try adding just the main dependency first.")
else:
return 0 # OK, status here is not used

if __name__ == "__main__":
main()