Skip to content

Add Edge-specific DEPS file pinning lib/modules to c637015f #1502

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.

Sign up for GitHub

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

Closed
Lakshmi Manaswi Karra (lkarra2) wants to merge 1 commit into from
Closed

Add Edge-specific DEPS file pinning lib/modules to c637015f #1502

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
21 changes: 21 additions & 0 deletions DEPS
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
#
# Copyright (c) Microsoft Corporation. All rights reserved.
# SPDX-License-Identifier: Apache-2.0
#

use_relative_paths = True

git_dependencies = 'DEPS'

@bmehta001 bmehta001 Jul 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

git_dependencies = 'DEPS' declares DEPS authoritative over git submodules for this repo. For a gclient consumer that recurses into this DEPS, the lib/modules submodule won''t be synced; combined with the condition-gated dep (once the False default above is added), a non-Edge gclient consumer would then fetch lib/modules from neither path. Harmless today (no other gclient consumers of this repo) and completely inert for non-gclient users, but worth a comment so the interaction is intentional.


vars = {
'edge_git': 'https://microsoft.visualstudio.com/edge/_git',
'edge_git_suffix': '',
'oneds_modules_revision': 'c637015fbbe904ed556d27e3b9072f6f2a5ee401',

@bmehta001 bmehta001 Jul 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This pins lib/modules to a hardcoded SHA on an Edge Azure DevOps mirror (edge_git), while .gitmodules points lib/modules at ../cpp_client_telemetry_modules.git (GitHub microsoft/cpp_client_telemetry_modules). That is two sources of truth on two different remotes, with nothing enforcing they stay in sync (the PR body notes "keep in sync" but it''s manual). Consider a CI check asserting oneds_modules_revision equals the submodule SHA, or a comment naming the submodule as canonical.

}

Comment thread
bmehta001 marked this conversation as resolved.
deps = {
'lib/modules': {
'url': Var('edge_git') + '/microsoft.cpp_client_telemetry_modules' + Var('edge_git_suffix') + '@' + Var('oneds_modules_revision'),
'condition': 'checkout_ms_src_internal'

@bmehta001 bmehta001 Jul 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

checkout_ms_src_internal is referenced here but is not defined in the vars block above. I traced this through depot_tools gclient_eval.py::EvaluateCondition: an undefined name in a condition is not a NameError (contrary to the earlier automated comment) — the code "implicitly converts unrecognized names to strings", yielding the non-empty (truthy) string 'checkout_ms_src_internal', and gclient.py::_should_process() returns that value directly, so the dep is processed.

Net effect: any gclient consumer that does not define this var will try to clone lib/modules from the Edge-internal microsoft.visualstudio.com/edge/_git/... URL and fail — the opposite of "no effect on other teams."

Fix — give it a False default so the safe path is "skip", and let Edge''s parent .gclient override it via custom_vars:

vars = {
  'edge_git': '...',
  'edge_git_suffix': '',
  'oneds_modules_revision': 'c637015f...',
  'checkout_ms_src_internal': False,   # Edge sets this True via custom_vars
}

},
}
Loading