Skip to content

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

Closed
Lakshmi Manaswi Karra (lkarra2) wants to merge 1 commit into
mainfrom
user/lkarra2/add-Edge-specific-DEPS
Closed

Add Edge-specific DEPS file pinning lib/modules to c637015f#1502
Lakshmi Manaswi Karra (lkarra2) wants to merge 1 commit into
mainfrom
user/lkarra2/add-Edge-specific-DEPS

Conversation

@lkarra2

@lkarra2 Lakshmi Manaswi Karra (lkarra2) commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Adds a top-level DEPS file so that gclient can resolve the lib/modules dependency (microsoft.cpp_client_telemetry_modules) when this SDK is consumed as a nested dependency of a larger gclient-managed source tree.

What it does

  • Pins lib/modules to revision c637015fbbe904ed556d27e3b9072f6f2a5ee401 via a deps entry.
  • Uses use_relative_paths = True so the entry resolves relative to this repo's checkout location.

Why
Downstream consumers that vendor this SDK via gclient need an explicit DEPS entry to fetch lib/modules; without it the module sources are missing and dependent code fails to build.

Notes

  • The pinned revision should be kept in sync with the lib/modules submodule pointer.

@lkarra2
Lakshmi Manaswi Karra (lkarra2) requested a review from a team as a code owner July 8, 2026 21:34
This Edge-only file mirrors the DEPS shipped on the m93 release branch and
is required for gclient to fetch lib/modules when this outer SDK revision
is referenced from chromium.src DEPS.edge. Without it, gn check fails on
public/lib/modules/utc/UtcTelemetrySystem.cpp and utc/desktop/UtcHelpers.cpp.

Copilot AI left a comment

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.

Pull request overview

Adds a top-level DEPS file so gclient consumers can fetch the lib/modules dependency when this SDK is included as a nested dependency in a larger gclient-managed checkout.

Changes:

  • Introduces a repository-root DEPS file with use_relative_paths = True.
  • Adds a deps['lib/modules'] entry pinned to revision c637015fbbe904ed556d27e3b9072f6f2a5ee401, gated by an internal-checkout condition.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread DEPS
Comment thread DEPS
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'

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
}

Comment thread DEPS

use_relative_paths = True

git_dependencies = 'DEPS'

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.

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

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.

@lkarra2

Copy link
Copy Markdown
Contributor Author

Abandoning the pull request for now, exploring option to add this file to Edge downstream branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants