Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
5 changes: 5 additions & 0 deletions eng/ingest-maven-deps.sh
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,11 @@ if [ -z "$TOKEN" ]; then
fi
echo "Token acquired."

# Force Gradle to use the Azure Artifacts feed (same as CI) so that

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.

💡 Minor (3/3 consensus after follow-up)

The comment explains what this does ("same as CI") but not why it's now required. The causal link — that settings.gradle conditionally gates the Azure Artifacts path on this variable — is non-obvious to someone reading this file in isolation.

Suggestion: Add a cross-reference:

# Force Gradle to use the Azure Artifacts feed (same as CI) so that
# dependency resolution goes through the feed and triggers ingestion.
# Required because settings.gradle gates repo selection on TF_BUILD.
export TF_BUILD=True

# dependency resolution goes through the feed and triggers ingestion.
# Required because settings.gradle gates repo selection on TF_BUILD.
export TF_BUILD=True

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.

💡 Suggestion (2/3 consensus after follow-up) — Consider moving this export closer to the top of the script (near other variable declarations). Since TF_BUILD governs all subsequent Gradle commands, placing it earlier makes the script's intent clear at a glance. Current placement is functionally correct but slightly buried after the token acquisition logic.


# Step 2: Ingest platform-specific artifacts for all OS variants
# Gradle only resolves the classifier for the current OS (e.g. aapt2-osx.jar on macOS).
# CI builds on Windows/Linux need their variants pre-ingested too.
Expand Down
11 changes: 8 additions & 3 deletions src/Core/AndroidNative/build.gradle
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
// Top-level build file where you can add configuration options common to all sub-projects/modules.
buildscript {
repositories {
maven {
url = 'https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-public-maven/maven/v1'
name = 'dotnet-public-maven'
if ((System.getenv('TF_BUILD') ?: '').equalsIgnoreCase('true')) {
maven {
url = 'https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-public-maven/maven/v1'
name = 'dotnet-public-maven'
}
} else {
google()
mavenCentral()
}
}
dependencies {
Expand Down
49 changes: 25 additions & 24 deletions src/Core/AndroidNative/settings.gradle
Original file line number Diff line number Diff line change
@@ -1,40 +1,41 @@
// Project Maven dependencies are resolved through the dnceng Azure Artifacts
// feed (dotnet-public-maven) for CFSClean network isolation compliance. The feed
// proxies Maven Central, Google Maven, and Gradle Plugin Portal. The credential
// provider plugin is fetched from a separate Azure Artifacts feed (artifacts-public).
// In CI (CFSClean network isolation), Maven dependencies resolve through the
// dnceng Azure Artifacts feed (dotnet-public-maven). Locally, standard Maven
// Central and Google Maven are used for faster builds.
//
// IMPORTANT: New packages must be ingested into the feed before CI can use them.
// The CI credential provider plugin skips auth in Azure Pipelines, so packages
// that aren't already in the feed will fail with 401. After adding or updating
// dependencies, run:
//
// ./eng/ingest-maven-deps.sh
// IMPORTANT: New packages must be ingested before CI can use them.
// If CI fails with "Could not GET ... 401", the package is not yet in the feed.
// Run ./eng/ingest-maven-deps.sh after adding or updating any Maven dependency.
//
// In CI, eng/init.gradle (injected by cache-gradle.yml) also handles
// project-level repo substitution for Android SDK binding targets.
// See: https://aka.ms/1es/netiso/CFS

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.

💡 Minor — Cross-reference to eng/init.gradle [2/3 consensus]

eng/init.gradle (injected by the CI pipeline template) handles project-level repository substitution via allprojects { repositories { ... } }, while this file handles settings-phase repos (pluginManagement, dependencyResolutionManagement). A brief cross-reference would help future maintainers understand the full CI repo-routing picture and avoid accidentally removing one half thinking it's redundant.


pluginManagement {

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.

🟢 Minor (2/3 consensus after follow-up) — pluginManagement is secondary to buildscript resolution

Currently, AGP resolution works because build.gradle's buildscript { dependencies { classpath "com.android.tools.build:gradle:..." } } puts AGP on the classpath directly. The pluginManagement.repositories block is not load-bearing for plugin resolution today.

If a future refactor removes the buildscript block (migrating fully to plugins {} DSL), this pluginManagement section becomes the sole CI mechanism for resolving AGP — and would depend on dotnet-public-maven proxying Google Maven correctly.

Suggestion: A brief comment noting this relationship would help future maintainers avoid accidentally breaking CI when modernizing the Gradle setup, e.g.:

// NOTE: pluginManagement repos are currently secondary — AGP resolves via
// buildscript.dependencies in build.gradle. If buildscript is removed,
// this block becomes load-bearing for CI plugin resolution.

repositories {
maven {
url = 'https://pkgs.dev.azure.com/artifacts-public/PublicTools/_packaging/AzureArtifacts/maven/v1'
name = 'AzureArtifacts'
}
maven {
url = 'https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-public-maven/maven/v1'
name = 'dotnet-public-maven'
if ((System.getenv('TF_BUILD') ?: '').equalsIgnoreCase('true')) {

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.

💡 Minor — CI plugin resolution depends on feed proxy coverage [2/3 consensus]

In CI mode, pluginManagement.repositories only has dotnet-public-maven. This works because the feed proxies Google Maven (which hosts AGP plugin markers like com.android.library). Consider a brief comment noting this dependency — if a future Gradle plugin is added whose marker isn't proxied by the feed, it will silently fail in CI without an obvious explanation.

maven {
url = 'https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-public-maven/maven/v1'
name = 'dotnet-public-maven'
}
} else {
google()
mavenCentral()
gradlePluginPortal()
}
}
}

plugins {
id 'com.microsoft.azure.artifacts.credprovider' version '1.1.1'
}

dependencyResolutionManagement {
repositoriesMode.set(RepositoriesMode.PREFER_PROJECT)
repositories {
maven {
url = 'https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-public-maven/maven/v1'
name = 'dotnet-public-maven'
if ((System.getenv('TF_BUILD') ?: '').equalsIgnoreCase('true')) {
maven {
url = 'https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-public-maven/maven/v1'
name = 'dotnet-public-maven'
}
} else {
google()
mavenCentral()
}
}
}
Expand Down
Loading