diff --git a/reports/PR-538-BREAKING-CHANGE-ANALYSIS.md b/reports/PR-538-BREAKING-CHANGE-ANALYSIS.md new file mode 100644 index 00000000..bab56d9d --- /dev/null +++ b/reports/PR-538-BREAKING-CHANGE-ANALYSIS.md @@ -0,0 +1,382 @@ +# PR #538 Breaking Change Analysis + +**PR**: https://github.com/jenkinsci/cloudbees-folder-plugin/pull/538 +**Date**: 2025-11-08 +**Status**: ⚠️ BREAKING CHANGE IDENTIFIED + +--- + +## Executive Summary + +The icon migration PR (#538) **will break** all branch source plugins (GitHub Branch Source, Bitbucket Branch Source, etc.) because they hardcode references to `"icon-folder"` and `"icon-folder-disabled"` CSS classes that this PR removes. + +**Impact**: GitHub Organizations, Bitbucket projects, and multibranch pipelines will show **black ×** instead of folder icons. + +--- + +## Root Cause + +### What the PR Changes + +**File**: `Folder.java` +**Changes**: IconSet registration migration + +```java +// BEFORE (current master) +IconSet.icons.addIcon(new Icon("icon-folder icon-sm", "plugin/cloudbees-folder/images/svgs/folder.svg", ...)); +IconSet.icons.addIcon(new Icon("icon-folder-disabled icon-sm", "plugin/cloudbees-folder/images/svgs/folder-disabled.svg", ...)); + +// AFTER (PR #538 migrate-icons-to-symbols branch) +IconSet.icons.addIcon(new Icon("symbol-folder icon-sm", "plugin/cloudbees-folder/images/svgs/folder.svg", ...)); +IconSet.icons.addIcon(new Icon("symbol-folder-disabled icon-sm", "plugin/cloudbees-folder/images/svgs/folder-disabled.svg", ...)); +``` + +### What Branch-API Plugin Depends On + +**Plugin**: branch-api-plugin (dependency of all branch source plugins) +**File**: `jenkins/branch/MetadataActionFolderIcon.java` +**Lines**: 74, 111 + +```java +@Override +public String getIconClassName() { + if (owner != null) { + if (owner.isDisabled()) { + return "icon-folder-disabled"; // ❌ HARDCODED STRING + } + // ... metadata action logic ... + } + return "icon-folder"; // ❌ HARDCODED STRING (fallback) +} +``` + +**Used by**: +- `OrganizationFolder` (GitHub Organizations, Bitbucket projects) +- `MultiBranchProject` (all multibranch pipelines) + +--- + +## Impact Assessment + +### Affected Plugins + +1. **branch-api-plugin** ⚠️ + - Returns hardcoded `"icon-folder"` and `"icon-folder-disabled"` + - Used by ALL branch source implementations + +2. **github-branch-source-plugin** ⚠️ + - GitHub Organizations use `MetadataActionFolderIcon` + - Will show black × for org folders + +3. **bitbucket-branch-source-plugin** ⚠️ + - Bitbucket projects use `MetadataActionFolderIcon` + - Will show black × for project folders + +4. **gitlab-branch-source-plugin** ⚠️ + - GitLab organizations use `MetadataActionFolderIcon` + - Will show black × for org folders + +5. **All multibranch pipelines** ⚠️ + - Use `MultiBranchProject` which extends branch-api + - Folder icons will break + +### User-Visible Impact + +**Before PR** (working): +``` +📁 My GitHub Organization + └─ 📁 repository-1 + └─ 📁 repository-2 +``` + +**After PR** (broken): +``` +❌ My GitHub Organization + └─ ❌ repository-1 + └─ ❌ repository-2 +``` + +**Severity**: HIGH - Very visible to users, affects core Jenkins workflow (GitHub orgs, multibranch) + +--- + +## Proposed Solutions + +### Option 1: Backward-Compatible Transition (RECOMMENDED) + +Register **BOTH** icon sets during transition period. + +**Change in `Folder.java`**: +```java +static { + // Modern symbol- icons (preferred) + IconSet.icons.addIcon(new Icon("symbol-folder icon-sm", "plugin/cloudbees-folder/images/svgs/folder.svg", Icon.ICON_SMALL_STYLE)); + IconSet.icons.addIcon(new Icon("symbol-folder icon-md", "plugin/cloudbees-folder/images/svgs/folder.svg", Icon.ICON_MEDIUM_STYLE)); + IconSet.icons.addIcon(new Icon("symbol-folder icon-lg", "plugin/cloudbees-folder/images/svgs/folder.svg", Icon.ICON_LARGE_STYLE)); + IconSet.icons.addIcon(new Icon("symbol-folder icon-xlg", "plugin/cloudbees-folder/images/svgs/folder.svg", Icon.ICON_XLARGE_STYLE)); + + IconSet.icons.addIcon(new Icon("symbol-folder-disabled icon-sm", "plugin/cloudbees-folder/images/svgs/folder-disabled.svg", Icon.ICON_SMALL_STYLE)); + IconSet.icons.addIcon(new Icon("symbol-folder-disabled icon-md", "plugin/cloudbees-folder/images/svgs/folder-disabled.svg", Icon.ICON_MEDIUM_STYLE)); + IconSet.icons.addIcon(new Icon("symbol-folder-disabled icon-lg", "plugin/cloudbees-folder/images/svgs/folder-disabled.svg", Icon.ICON_LARGE_STYLE)); + IconSet.icons.addIcon(new Icon("symbol-folder-disabled icon-xlg", "plugin/cloudbees-folder/images/svgs/folder-disabled.svg", Icon.ICON_XLARGE_STYLE)); + + // Deprecated icon- classes (backward compatibility for branch-api-plugin) + // TODO: Remove after branch-api-plugin migrates to symbol- format + IconSet.icons.addIcon(new Icon("icon-folder icon-sm", "plugin/cloudbees-folder/images/svgs/folder.svg", Icon.ICON_SMALL_STYLE)); + IconSet.icons.addIcon(new Icon("icon-folder icon-md", "plugin/cloudbees-folder/images/svgs/folder.svg", Icon.ICON_MEDIUM_STYLE)); + IconSet.icons.addIcon(new Icon("icon-folder icon-lg", "plugin/cloudbees-folder/images/svgs/folder.svg", Icon.ICON_LARGE_STYLE)); + IconSet.icons.addIcon(new Icon("icon-folder icon-xlg", "plugin/cloudbees-folder/images/svgs/folder.svg", Icon.ICON_XLARGE_STYLE)); + + IconSet.icons.addIcon(new Icon("icon-folder-disabled icon-sm", "plugin/cloudbees-folder/images/svgs/folder-disabled.svg", Icon.ICON_SMALL_STYLE)); + IconSet.icons.addIcon(new Icon("icon-folder-disabled icon-md", "plugin/cloudbees-folder/images/svgs/folder-disabled.svg", Icon.ICON_MEDIUM_STYLE)); + IconSet.icons.addIcon(new Icon("icon-folder-disabled icon-lg", "plugin/cloudbees-folder/images/svgs/folder-disabled.svg", Icon.ICON_LARGE_STYLE)); + IconSet.icons.addIcon(new Icon("icon-folder-disabled icon-xlg", "plugin/cloudbees-folder/images/svgs/folder-disabled.svg", Icon.ICON_XLARGE_STYLE)); +} +``` + +**Pros**: +- ✅ No breaking changes - both icon sets work +- ✅ Can merge folder-plugin PR immediately +- ✅ Gives time for branch-api-plugin to migrate +- ✅ Safe rollout strategy + +**Cons**: +- ⚠️ Temporary code duplication (12 extra lines) +- ⚠️ Requires follow-up PR to clean up deprecated icons + +**Timeline**: +1. **Week 1**: Merge folder-plugin with both icon sets +2. **Week 2-4**: Update branch-api-plugin to use `symbol-folder` +3. **Week 5**: Remove deprecated `icon-*` registrations from folder-plugin + +--- + +### Option 2: Coordinated PRs (Complex) + +Update branch-api-plugin first, then folder-plugin. + +**PR 1 (branch-api-plugin)**: +```java +// File: jenkins/branch/MetadataActionFolderIcon.java +@Override +public String getIconClassName() { + if (owner != null) { + if (owner.isDisabled()) { + return "symbol-folder-disabled"; // Changed + } + // ... metadata action logic ... + } + return "symbol-folder"; // Changed +} +``` + +**PR 2 (folder-plugin)**: Current PR #538 (no changes needed) + +**Pros**: +- ✅ Clean migration, no temporary duplication +- ✅ Modern approach + +**Cons**: +- ❌ Requires branch-api-plugin PR approval first +- ❌ Delays folder-plugin merge +- ❌ Coordination overhead across 2 repos +- ❌ Risk: What if branch-api PR stalls? + +--- + +### Option 3: Convert to Non-Breaking Change (Safe Alternative) + +Keep ONLY Jelly migrations, don't change IconSet registrations. + +**Keep `icon-*` registrations in Folder.java**, only update Jelly files to use modern syntax: +```xml + + +``` + +But keep Java IconSet registrations as `icon-folder` for backward compatibility. + +**Pros**: +- ✅ Jelly files use modern patterns +- ✅ No breaking changes to dependent plugins +- ✅ Can merge immediately + +**Cons**: +- ⚠️ Incomplete migration (Java still uses deprecated names) +- ⚠️ Doesn't fully solve the icon deprecation issue + +--- + +## Recommendation + +**Use Option 1: Backward-Compatible Transition** + +### Migration Plan + +#### Phase 1: Folder Plugin (This PR) +1. **Update PR #538** to register BOTH icon sets +2. **Add deprecation comment** in code +3. **Update PR description** with migration timeline +4. **Get approval** from maintainers (@alecharp, @rsandell, @jglick) +5. **Merge** folder-plugin with both icon sets + +#### Phase 2: Branch-API Plugin (New PR) +1. **Create PR** in branch-api-plugin +2. **Update** `MetadataActionFolderIcon.java` to return `symbol-folder*` +3. **Test** with GitHub Branch Source, Bitbucket Branch Source +4. **Merge** branch-api-plugin + +#### Phase 3: Cleanup (Follow-up PR) +1. **Wait** for branch-api-plugin adoption (1-2 months) +2. **Create PR** in folder-plugin to remove deprecated `icon-*` registrations +3. **Update release notes** about icon deprecation removal + +--- + +## Testing Requirements + +### Before Merging PR #538 + +1. **Build folder-plugin** with backward-compatible fix +2. **Start test Jenkins** with: + - GitHub Branch Source Plugin + - Bitbucket Branch Source Plugin +3. **Create**: + - GitHub Organization folder + - Bitbucket project folder + - Regular folder (baseline) +4. **Verify icons display correctly** (no black ×) +5. **Screenshot evidence** for PR + +### After Branch-API Update + +1. **Build branch-api-plugin** with symbol- migration +2. **Test with folder-plugin** (both old and new) +3. **Verify no regressions** + +--- + +## Files to Update + +### Immediate (PR #538) + +- `src/main/java/com/cloudbees/hudson/plugins/folder/Folder.java` + - Add backward-compatible icon registrations + - Add deprecation comment + +### Future (Branch-API PR) + +- `branch-api-plugin/src/main/java/jenkins/branch/MetadataActionFolderIcon.java` + - Lines 74, 111: Change to `symbol-folder*` + +### Cleanup (Future PR) + +- `src/main/java/com/cloudbees/hudson/plugins/folder/Folder.java` + - Remove deprecated `icon-*` registrations + +--- + +## Communication Plan + +### Update PR #538 Comment + +Post findings and proposed solution to PR: + +```markdown +## 🚨 Breaking Change Identified + +After investigation, I've confirmed that merging this PR **as-is would break**: +- GitHub Organizations +- Bitbucket projects +- All multibranch pipelines + +### Root Cause + +The branch-api-plugin hardcodes `"icon-folder"` and `"icon-folder-disabled"` in `MetadataActionFolderIcon.java` (lines 74, 111). These CSS classes would no longer exist after this PR. + +### Proposed Solution + +I'll update this PR to register **BOTH** icon sets during the transition: +- New: `symbol-folder*` (preferred) +- Deprecated: `icon-folder*` (backward compatibility) + +This allows: +1. ✅ Merge this PR safely (no breaking changes) +2. ✅ Time for branch-api-plugin to migrate +3. ✅ Clean up deprecated icons in future PR + +### Testing Plan + +- Setting up test Jenkins with GitHub Branch Source +- Will verify icons display correctly +- Screenshots coming soon + +Detailed analysis: [link to report] +``` + +### Contact Branch-API Maintainers + +- Open issue in branch-api-plugin repository +- Link to this analysis +- Propose PR timeline +- Coordinate release schedule + +--- + +## Risk Assessment + +### High Risk (Current PR without fix) + +- ⚠️ Breaks GitHub orgs, Bitbucket projects, multibranch pipelines +- ⚠️ Very visible to users +- ⚠️ Could block Jenkins upgrades +- ⚠️ Reputation risk for thoroughness + +### Low Risk (With backward-compatible fix) + +- ✅ No breaking changes +- ✅ Smooth migration path +- ✅ Time to coordinate with branch-api +- ✅ Safe rollout + +--- + +## Timeline Estimate + +| Phase | Duration | Status | +|-------|----------|--------| +| Document findings | 2 hours | ✅ Complete | +| Update PR #538 with backward-compat fix | 1 hour | ⏳ Next | +| Test with branch source plugins | 2 hours | ⏳ Pending | +| Get PR approval | 1-3 days | ⏳ Pending | +| Merge folder-plugin | 1 day | ⏳ Pending | +| Create branch-api PR | 2 hours | 📅 Week 2 | +| Branch-api review & merge | 1-2 weeks | 📅 Week 3-4 | +| Cleanup deprecated icons | 1 hour | 📅 Week 5+ | + +--- + +## References + +### PRs +- **Folder Plugin**: https://github.com/jenkinsci/cloudbees-folder-plugin/pull/538 + +### Source Files +- **Folder.java**: `/src/jenkins/plugins/cloudbees-folder-plugin/src/main/java/com/cloudbees/hudson/plugins/folder/Folder.java` +- **MetadataActionFolderIcon.java**: `/src/jenkins/plugins/branch-api-plugin/src/main/java/jenkins/branch/MetadataActionFolderIcon.java` +- **OrganizationFolder.java**: `/src/jenkins/plugins/branch-api-plugin/src/main/java/jenkins/branch/OrganizationFolder.java` + +### Related Work +- Icon migration patterns: `/src/jenkins/jenkins-core-monitor/docs/migrations/ICON-PATTERNS.md` +- Jenkins core PR: https://github.com/jenkinsci/jenkins/pull/10245 + +--- + +**Analysis by**: Thorsten Scherler (@scherler) +**Date**: 2025-11-08 +**Severity**: HIGH (breaking change) +**Recommendation**: Implement backward-compatible transition (Option 1) + +🤖 Generated with [Claude Code](https://claude.com/claude-code) + +Co-Authored-By: Claude diff --git a/src/main/java/com/cloudbees/hudson/plugins/folder/Folder.java b/src/main/java/com/cloudbees/hudson/plugins/folder/Folder.java index fb4b84b6..62e4fc6a 100644 --- a/src/main/java/com/cloudbees/hudson/plugins/folder/Folder.java +++ b/src/main/java/com/cloudbees/hudson/plugins/folder/Folder.java @@ -434,7 +434,23 @@ public TopLevelItem newInstance(ItemGroup parent, String name) { } static { - // fix the IconSet defaults because some of them are .gif files and icon-folder should really be here and not in core + // Modern symbol- icons (preferred) + IconSet.icons.addIcon(new Icon("symbol-folder icon-sm", "plugin/cloudbees-folder/images/svgs/folder.svg", Icon.ICON_SMALL_STYLE)); + IconSet.icons.addIcon(new Icon("symbol-folder icon-md", "plugin/cloudbees-folder/images/svgs/folder.svg", Icon.ICON_MEDIUM_STYLE)); + IconSet.icons.addIcon(new Icon("symbol-folder icon-lg", "plugin/cloudbees-folder/images/svgs/folder.svg", Icon.ICON_LARGE_STYLE)); + IconSet.icons.addIcon(new Icon("symbol-folder icon-xlg", "plugin/cloudbees-folder/images/svgs/folder.svg", Icon.ICON_XLARGE_STYLE)); + IconSet.icons.addIcon(new Icon("symbol-folder-disabled icon-sm", "plugin/cloudbees-folder/images/svgs/folder-disabled.svg", Icon.ICON_SMALL_STYLE)); + IconSet.icons.addIcon(new Icon("symbol-folder-disabled icon-md", "plugin/cloudbees-folder/images/svgs/folder-disabled.svg", Icon.ICON_MEDIUM_STYLE)); + IconSet.icons.addIcon(new Icon("symbol-folder-disabled icon-lg", "plugin/cloudbees-folder/images/svgs/folder-disabled.svg", Icon.ICON_LARGE_STYLE)); + IconSet.icons.addIcon(new Icon("symbol-folder-disabled icon-xlg", "plugin/cloudbees-folder/images/svgs/folder-disabled.svg", Icon.ICON_XLARGE_STYLE)); + IconSet.icons.addIcon(new Icon("symbol-folder-store icon-sm", "plugin/cloudbees-folder/images/svgs/folder-store.svg", Icon.ICON_SMALL_STYLE)); + IconSet.icons.addIcon(new Icon("symbol-folder-store icon-md", "plugin/cloudbees-folder/images/svgs/folder-store.svg", Icon.ICON_MEDIUM_STYLE)); + IconSet.icons.addIcon(new Icon("symbol-folder-store icon-lg", "plugin/cloudbees-folder/images/svgs/folder-store.svg", Icon.ICON_LARGE_STYLE)); + IconSet.icons.addIcon(new Icon("symbol-folder-store icon-xlg", "plugin/cloudbees-folder/images/svgs/folder-store.svg", Icon.ICON_XLARGE_STYLE)); + + // Deprecated icon- classes (backward compatibility for branch-api-plugin and dependent plugins) + // TODO: Remove these after branch-api-plugin migrates to symbol- format + // See: https://github.com/jenkinsci/branch-api-plugin (MetadataActionFolderIcon.java lines 74, 111) IconSet.icons.addIcon(new Icon("icon-folder icon-sm", "plugin/cloudbees-folder/images/svgs/folder.svg", Icon.ICON_SMALL_STYLE)); IconSet.icons.addIcon(new Icon("icon-folder icon-md", "plugin/cloudbees-folder/images/svgs/folder.svg", Icon.ICON_MEDIUM_STYLE)); IconSet.icons.addIcon(new Icon("icon-folder icon-lg", "plugin/cloudbees-folder/images/svgs/folder.svg", Icon.ICON_LARGE_STYLE)); diff --git a/src/main/resources/com/cloudbees/hudson/plugins/folder/AbstractFolder/tasks-new.jelly b/src/main/resources/com/cloudbees/hudson/plugins/folder/AbstractFolder/tasks-new.jelly index 0772e21e..e06a50d5 100644 --- a/src/main/resources/com/cloudbees/hudson/plugins/folder/AbstractFolder/tasks-new.jelly +++ b/src/main/resources/com/cloudbees/hudson/plugins/folder/AbstractFolder/tasks-new.jelly @@ -28,6 +28,6 @@ THE SOFTWARE. - + diff --git a/src/main/resources/com/cloudbees/hudson/plugins/folder/AbstractFolder/tasks-top.jelly b/src/main/resources/com/cloudbees/hudson/plugins/folder/AbstractFolder/tasks-top.jelly index dc625d64..41410ee7 100644 --- a/src/main/resources/com/cloudbees/hudson/plugins/folder/AbstractFolder/tasks-top.jelly +++ b/src/main/resources/com/cloudbees/hudson/plugins/folder/AbstractFolder/tasks-top.jelly @@ -28,10 +28,10 @@ THE SOFTWARE. - + - + diff --git a/src/main/resources/com/cloudbees/hudson/plugins/folder/Folder/tasks-create.jelly b/src/main/resources/com/cloudbees/hudson/plugins/folder/Folder/tasks-create.jelly index a8240d99..4c321df5 100644 --- a/src/main/resources/com/cloudbees/hudson/plugins/folder/Folder/tasks-create.jelly +++ b/src/main/resources/com/cloudbees/hudson/plugins/folder/Folder/tasks-create.jelly @@ -24,5 +24,5 @@ THE SOFTWARE. - + diff --git a/src/main/resources/com/cloudbees/hudson/plugins/folder/computed/ComputedFolder/tasks-top-extra.jelly b/src/main/resources/com/cloudbees/hudson/plugins/folder/computed/ComputedFolder/tasks-top-extra.jelly index 23c1c0db..4383b481 100644 --- a/src/main/resources/com/cloudbees/hudson/plugins/folder/computed/ComputedFolder/tasks-top-extra.jelly +++ b/src/main/resources/com/cloudbees/hudson/plugins/folder/computed/ComputedFolder/tasks-top-extra.jelly @@ -28,12 +28,12 @@ THE SOFTWARE. - + - - + + - + diff --git a/src/main/resources/com/cloudbees/hudson/plugins/folder/computed/FolderComputation/index.jelly b/src/main/resources/com/cloudbees/hudson/plugins/folder/computed/FolderComputation/index.jelly index 3092b2e3..6076ed03 100644 --- a/src/main/resources/com/cloudbees/hudson/plugins/folder/computed/FolderComputation/index.jelly +++ b/src/main/resources/com/cloudbees/hudson/plugins/folder/computed/FolderComputation/index.jelly @@ -34,7 +34,7 @@ THE SOFTWARE. - + ${%Not run}