fix(#4756): derive builder.resources.excludes for ABAP deploy at runtime#4828
Open
longieirl wants to merge 57 commits into
Open
fix(#4756): derive builder.resources.excludes for ABAP deploy at runtime#4828longieirl wants to merge 57 commits into
longieirl wants to merge 57 commits into
Conversation
…onfig Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…s, add chainability test
…eployTask/addCloudFoundryDeployTask Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…i5.yaml via addBuilderResourceExcludes Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…udes() before deploy task methods
…angeset Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…f-contained Call addBuilderResourceExcludes() inside the deploy task methods so callers do not need to call it explicitly. Remove redundant explicit calls from adp-tooling, cf-deploy-config-writer, and abap-deploy-config-writer getDeployConfig. updateBaseConfig retains its call (only source of excludes for base ui5.yaml). updateBaseConfig now skips fs.write when content unchanged. Adds test for malformed excludes: null.
Combined HEAD's idempotent addBuilderResourceExcludes() with main's deployExclude logic (#4757) that injects /test/ and /localService/ into the ABAP task exclude config. Updated two abap-deploy-config-writer snapshots to include builder.resources.excludes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ources.excludes addCloudFoundryDeployTask now calls addBuilderResourceExcludes() internally, adding builder.resources.excludes to generated ui5.yaml snapshots. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…configuration.exclude from addAbapDeployTask
…e from getDeployConfig
…nfiguration.exclude removed from new configs
…tion.exclude at runtime Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
….yaml Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…nts; strengthen test assertions Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…ed ui5.yaml Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…ration Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…ill generated by deploy task Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…cludes added to generated ui5.yaml Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…er.resources.excludes added to generated ui5.yaml Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…excludes added to generated ui5.yaml Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…cal.yaml; strengthen test assertions; fix changeset prefix Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…eDoS-prone regex, improve coverage, restore snapshot ordering - Replace /\/?\*+$/ regex with safe string ops in addAbapDeployTask - Add test covering exclude-branch in cli/createArchiveFromFolder (lines 65-66) - Rebuild ui5-config and regenerate abap-deploy-config-writer snapshot so field order (exclude before lrep) matches main Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…xclude reach getArchive Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…mFolder Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
![hyperspace-insights[bot]](https://avatars.githubusercontent.com/u/2531208?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
The PR introduces a useful feature but has several correctness issues that need to be addressed before merging: the changeset summary will fail CI validation, the RegExp with the 'g' flag in the archive exclusion loop is both unnecessary and produces unreliable results, the glob-normalisation logic in addAbapDeployTask can silently mangle non-standard patterns like /assets/my*, and the CLI --exclude flag's interaction with YAML-configured excludes (override vs. additive) is ambiguous and potentially lossy.
…e matching - addAbapDeployTask no longer writes configuration.exclude; runtime derives from builder.resources.excludes in the deploy config - mergeConfig merges all three sources: CLI --exclude, YAML configuration.exclude, and builder.resources.excludes (deduplicated) - fix archive exclusion: startsWith with leading-slash strip replaces broken RegExp approach - fix changeset prefix: fix: -> FIX: - add fixtures and tests for new exclude flow Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…-resource-excludes-derive
Replace while-loop star-stripping with p.replace(/\/\*+$/, '/') to avoid mangling patterns like /assets/my* into /assets/my/ (an incorrect dir prefix). Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…uration.exclude addAbapDeployTask no longer writes configuration.exclude by default; update toStrictEqual assertions and snapshots in abap-deploy-config-sub-generator, abap-deploy-config-writer, and deploy-config-sub-generator to reflect this. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Contributor
Author
These comments have beeb addressed. |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #4756
Summary
Fixes a silent misalignment between build-time and deploy-time exclusions: builder.resources.excludes in ui5-deploy.yaml was correctly set but never read by the deploy task, which maintained its own hardcoded configuration.exclude list. As a result, folders like localService/ were excluded at build time but still
uploaded during ABAP deployment.
Changes:
Test plan