Use Azure Artifacts feed only in CI, standard repos locally#35219
Use Azure Artifacts feed only in CI, standard repos locally#35219
Conversation
The credential provider plugin and Azure Artifacts feed add significant overhead locally (~10+ min builds reported by team). Use google()/mavenCentral() for local builds and only switch to the Azure Artifacts feed when TF_BUILD=True (Azure Pipelines). The init.gradle still handles Android SDK bindings Gradle targets redirection in CI via the pipeline template. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
🚀 Dogfood this PR with:
curl -fsSL https://github.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 35219Or
iex "& { $(irm https://github.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 35219" |
There was a problem hiding this comment.
Pull request overview
Updates the AndroidNative Gradle configuration to use standard public Maven repositories for local development builds while reserving the Azure Artifacts dotnet-public-maven feed for CI (CFSClean / network isolation) scenarios.
Changes:
- Conditionally selects Maven repositories based on
TF_BUILDto avoid Azure Artifacts + credential-provider overhead locally. - Removes the unconditional Azure Artifacts plugin feed/credential-provider setup from
settings.gradle. - Applies the same conditional repository logic to the top-level
buildscriptrepositories.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/Core/AndroidNative/settings.gradle | Switches plugin/dependency repositories between CI (Azure Artifacts) and local (google/mavenCentral/plugin portal). |
| src/Core/AndroidNative/build.gradle | Switches buildscript repositories between CI (Azure Artifacts) and local (google/mavenCentral). |
| if (System.getenv('TF_BUILD') == 'True') { | ||
| maven { | ||
| url = 'https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-public-maven/maven/v1' | ||
| name = 'dotnet-public-maven' | ||
| } | ||
| } else { | ||
| google() | ||
| mavenCentral() | ||
| gradlePluginPortal() |
There was a problem hiding this comment.
The Azure Artifacts feed is now only enabled when TF_BUILD == 'True', but eng/ingest-maven-deps.sh runs locally and relies on these Gradle files to resolve through dotnet-public-maven to ingest missing packages. With this condition, the ingest script will instead use google()/mavenCentral() unless the developer manually sets TF_BUILD, so it won’t pre-populate the feed as intended. Consider adding a separate opt-in env var (e.g., DOTNET_PUBLIC_MAVEN_INGEST=true) that also enables the feed locally, and update the ingest script/docs to set it when running ingestion.
| if (System.getenv('TF_BUILD') == 'True') { | ||
| maven { | ||
| url = 'https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-public-maven/maven/v1' | ||
| name = 'dotnet-public-maven' |
There was a problem hiding this comment.
System.getenv('TF_BUILD') == 'True' is a very strict check; other CI environments or future Azure Pipelines changes may set TF_BUILD to a different truthy value (e.g., true, 1). Using a presence check (!= null) or a case-insensitive boolean parse would make the CI/local repo selection more robust.
| if (System.getenv('TF_BUILD') == 'True') { | ||
| maven { | ||
| url = 'https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-public-maven/maven/v1' | ||
| name = 'dotnet-public-maven' | ||
| } | ||
| } else { | ||
| google() | ||
| mavenCentral() | ||
| } |
There was a problem hiding this comment.
Same as in settings.gradle: gating the Azure Artifacts repository solely on TF_BUILD == 'True' makes eng/ingest-maven-deps.sh ineffective when run locally (it will resolve from google()/mavenCentral() instead of the feed, so it won’t ingest). Add an explicit local override env var (used by the ingest script) so developers can intentionally run against dotnet-public-maven without pretending to be in CI.
| if (System.getenv('TF_BUILD') == 'True') { | ||
| maven { | ||
| url = 'https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-public-maven/maven/v1' | ||
| name = 'dotnet-public-maven' | ||
| } |
There was a problem hiding this comment.
As above, System.getenv('TF_BUILD') == 'True' is brittle. Prefer a presence check or case-insensitive boolean parse so CI reliably selects the Azure Artifacts feed.
This comment has been minimized.
This comment has been minimized.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Expert Code Review: 4 findings posted inline (1 critical, 2 moderate, 1 minor). See summary comment for full details and methodology.
Generated by Expert Code Review · ● 14.2M
| maven { | ||
| url = 'https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-public-maven/maven/v1' | ||
| name = 'dotnet-public-maven' | ||
| if (System.getenv('TF_BUILD') == 'True') { |
There was a problem hiding this comment.
🟡 Moderate — 2/3 consensus | Case-sensitive string comparison is brittle
System.getenv('TF_BUILD') == 'True' is case-sensitive in Groovy. While Azure Pipelines documents the value as exactly True, any deviation (e.g., a wrapper script exporting TF_BUILD=true or TRUE) will silently fall through to google()/mavenCentral(). In a CFSClean environment, this causes an opaque 401 failure with no diagnostic.
Recommendation: Normalize once and compare case-insensitively:
def isCi = (System.getenv('TF_BUILD') ?: '').equalsIgnoreCase('true')Optionally add a logger.lifecycle("Gradle repo routing: CI=\$\{isCi}") to make the routing decision visible in build logs.
| // dnceng Azure Artifacts feed (dotnet-public-maven). Locally, standard Maven | ||
| // Central and Google Maven are used for faster builds. | ||
| // | ||
| // When adding or updating dependencies, run eng/ingest-maven-deps.sh to |
There was a problem hiding this comment.
🔴 Critical — 3/3 consensus | eng/ingest-maven-deps.sh is broken by this change
The ingestion script runs ./gradlew build --refresh-dependencies locally (without TF_BUILD=True). After this PR, Gradle resolves from google()/mavenCentral() instead of the Azure Artifacts feed, so the core ingestion mechanism (pulling through the feed to trigger upstream caching) no longer works.
Additionally, the -Dazure.artifacts.credprovider.* system properties passed by the script are now dead code since the credential provider plugin was removed.
Impact: The next time a dependency is added, running eng/ingest-maven-deps.sh will appear to succeed (Gradle builds fine from Maven Central) but won't populate the Azure Artifacts feed. CI will then fail with 401 errors.
Recommendation: Update eng/ingest-maven-deps.sh to export TF_BUILD=True before invoking Gradle, so the script forces resolution through the Azure Artifacts feed regardless of environment.
| maven { | ||
| url = 'https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-public-maven/maven/v1' | ||
| name = 'dotnet-public-maven' | ||
| if (System.getenv('TF_BUILD') == 'True') { |
There was a problem hiding this comment.
🟢 Minor — 2/3 consensus | Duplicated conditional logic increases drift risk
The if (System.getenv('TF_BUILD') == 'True') pattern is repeated in 3 places across 2 files. Each serves a different Gradle scope (pluginManagement, buildscript, dependencyResolutionManagement) but they look identical with no indication of their distinct roles or backup mechanisms.
Recommendation: Extract to a variable at the top of each file:
def isCi = (System.getenv('TF_BUILD') ?: '').equalsIgnoreCase('true')Then reuse isCi in all repository blocks. This also addresses the case-sensitivity concern above.
| maven { | ||
| url = 'https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-public-maven/maven/v1' | ||
| name = 'dotnet-public-maven' | ||
| if (System.getenv('TF_BUILD') == 'True') { |
There was a problem hiding this comment.
🟡 Moderate — 2/3 consensus | Non-obvious interaction with eng/init.gradle
In CI, init.gradle injects dotnet-public-maven as a project-level repo via allprojects { repositories { ... } }. Combined with PREFER_PROJECT mode, the init.gradle injection takes precedence — meaning this TF_BUILD conditional in dependencyResolutionManagement is actually a fallback, not the primary mechanism for project dependency resolution in CI.
Meanwhile, for pluginManagement.repositories and buildscript.repositories (lines 11 and build.gradle:4), init.gradle cannot reach those pre-project-evaluation contexts, so the TF_BUILD conditional there IS the sole CI guard.
This asymmetry is invisible to future maintainers.
Recommendation: Add a clarifying comment:
// NOTE: In CI, project-level repos injected by eng/init.gradle (via allprojects)
// take precedence (PREFER_PROJECT mode). This block is a safety net.
// For buildscript and pluginManagement, TF_BUILD is the sole CI guard.
- eng/ingest-maven-deps.sh now exports TF_BUILD=True so Gradle resolves through the Azure Artifacts feed during ingestion - Use equalsIgnoreCase for TF_BUILD check to handle any casing Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Expert Code Review: 5 findings (2 moderate, 3 minor). 3 posted inline on settings.gradle; 2 overflow findings (pre-existing lines made stale by this PR) in summary comment.
Generated by Expert Code Review · ● 14.2M
| // Central and Google Maven are used for faster builds. | ||
| // | ||
| // When adding or updating dependencies, run eng/ingest-maven-deps.sh to | ||
| // pre-populate the Azure Artifacts feed for CI. |
There was a problem hiding this comment.
The old comment block documented the specific CI failure mode: uningested packages fail with 401, and ingest-maven-deps.sh must be run to fix it. The new comment mentions the script but not why it's needed — the feed doesn't proxy on-demand, so new packages silently fail in CI until ingested.
Recommendation: Add a line explaining the failure mode, e.g.:
// If CI fails with "Could not GET ... 401", the package is not yet in the feed.
// Run eng/ingest-maven-deps.sh to pre-populate it.| // | ||
| // When adding or updating dependencies, run eng/ingest-maven-deps.sh to | ||
| // pre-populate the Azure Artifacts feed for CI. | ||
| // See: https://aka.ms/1es/netiso/CFS |
There was a problem hiding this comment.
💡 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.
| 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')) { |
There was a problem hiding this comment.
💡 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.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Expert Code Review: 3 findings (1 moderate, 2 minor). 2 posted inline; 1 in summary comment (lines not in diff).
Generated by Expert Code Review · ● 10.8M
| fi | ||
| echo "Token acquired." | ||
|
|
||
| # Force Gradle to use the Azure Artifacts feed (same as CI) so that |
There was a problem hiding this comment.
💡 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| // dnceng Azure Artifacts feed (dotnet-public-maven). Locally, standard Maven | ||
| // Central and Google Maven are used for faster builds. | ||
| // | ||
| // When adding or updating dependencies, run eng/ingest-maven-deps.sh to |
There was a problem hiding this comment.
💡 Minor (2/3 consensus)
The old comment included IMPORTANT: New packages must be ingested into the feed before CI can use them — with the credential provider plugin now removed, CI has no authenticated fallback for un-ingested packages (anonymous 401 with no helpful error). The "IMPORTANT" language was arguably more warranted now.
Suggestion: Consider restoring a stronger warning, e.g.:
// IMPORTANT: New packages must be ingested before CI can use them.
// Run ./eng/ingest-maven-deps.sh after adding or updating any Maven dependency.Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Expert Code Review: 2 findings (1 moderate, 1 minor). 1 posted inline; 1 in summary comment (lines not in diff hunk).
Generated by Expert Code Review · ● 13.3M
| // project-level repo substitution for Android SDK binding targets. | ||
| // See: https://aka.ms/1es/netiso/CFS | ||
|
|
||
| pluginManagement { |
There was a problem hiding this comment.
🟢 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.
Expert Code Review — PR #35219Methodology: 3 independent reviewers with adversarial consensus Result: 3 findings (1 moderate Overflow Findings (not in diff hunks)
Discarded Findings (did not reach consensus)
CI Status✅ 28/31 checks passing, 2 in progress (Build Analysis, Build Windows integration tests), 1 queued (maui-pr overall). No failures. Test CoverageThis PR modifies Gradle build infrastructure ( SummaryThe approach is architecturally sound. The
Warning
|
There was a problem hiding this comment.
Expert Code Review: 3 findings (1 moderate, 2 minor). 1 posted inline; 2 could not be posted inline (pre-existing lines not in diff) — see summary comment for details.
Warning
⚠️ Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
patchdiff.githubusercontent.com
To allow these domains, add them to the network.allowed list in your workflow frontmatter:
network:
allowed:
- defaults
- "patchdiff.githubusercontent.com"See Network Configuration for more information.
Generated by Expert Code Review · ● 15.4M
| # 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 |
There was a problem hiding this comment.
💡 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.
Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Problem
PR #35169 switched all Gradle repos to the Azure Artifacts feed unconditionally. This causes 10+ minute local Android builds because the credential provider plugin adds significant auth overhead (MSAL token acquisition, feed latency vs direct Maven Central).
Reported by team members in India experiencing builds that never complete.
Fix
Use
google()/mavenCentral()for local builds and only switch to the Azure Artifacts feed whenTF_BUILD=True(Azure Pipelines with CFSClean network isolation).This also removes the credential provider plugin dependency for local builds since it's not needed when using standard Maven repos.
The
init.gradlestill handles the Android SDK bindings Gradle targets redirection in CI via the pipeline template.Verified
TF_BUILD=Truewhich activates the Azure Artifacts feed