-
Notifications
You must be signed in to change notification settings - Fork 306
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
Changes from all commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
41a7353
Add new python step to pipeline
nagilson 883fe50
add dependency-verifier
nagilson 25cbcf7
Check if git can be called in pipeline
nagilson d679698
Use var syntax
nagilson 0601849
Fetch branches beforehand
nagilson ce44e69
PushGitBranchesToCheckRemote
nagilson a100f12
Update checks to branches in azdo
nagilson bc5ecd4
Yarn lock checker
nagilson 143611c
Add code to grab dependencies
nagilson 7fdd33d
Improve code quality
nagilson cab88da
Bug fixes
nagilson 6e7e68e
Several fixes to depedency verifier
nagilson d4d543a
pull nongeneric branch now that testing is done
nagilson 4b9820d
undo sudo testing install
nagilson 0bd81aa
Add warning that origin/targetbranch must be up to date
nagilson 6499c84
Merge branch 'main' into nagilson-python-azure-pipelines
nagilson 08c3415
Merge branch 'main' into nagilson-python-azure-pipelines
nagilson File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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() |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What specific features are you referring to?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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