Open Lightning00Blade opened 10 months ago
rowanmanning
commented
9 months ago I'm also seeing this issue and managed to reproduce locally with the CLI. If it'd be useful then I can try and push a reduced test case somewhere. I'm seeing it in both a brand new project and an existing one that I tried migrating to the latest version of Release Please.
Lightning00Blade
commented
9 months ago @rowanmanning please do, I would be happy to take a look myself if I have a repo.
01Joseph-Hwang10
commented
5 months ago I'm also having the same issue with this. So I did some experiment and debugging on it.
I reproduced this problem with several repos linked below.
Turned out this problem happened on 01Joseph-Hwang10/release-please-standalone-example#1 and 01Joseph-Hwang10/release-please-monorepo-without-root-example#1.
As I first faced this issue in standalone node project environment, I did some debugging in my local machine with release-please CLI. I added some more console.logs in the source of release-please to figure out what's happening and what's wrong.
What I ran:
release-please release-pr --token="$GITHUB_TOKEN" --repo-url="https://github.com/01Joseph-Hwang10/release-please-standalone-example" --trace --dry-runThe below was the output. Note that there exist several >>>>>>>>>>>> START LOGS @ <class & method names> >>>>>>>>>>>> lines to express that this is console.log lines I added for debugging purposes.
I found out @ class ReleasePleaseManifest.updateContent part, which appears 2 times among the output,
and found out each of those are
releasePullRequest.updates.push(...) at class Manifest.buildPullRequests and newUpdates.push(...) at abstract class WorkspacePlugin.run.Those updates are pushed in order I enumerated above. So the firstly pushed release please manifest update corresponds with the logs:
>>>>>>>>>>>>>>>>>>>> START LOGS @ class ReleasePleaseManifest.updateContent >>>>>>>>>>>>>>>>>>>>
content: {
".": "0.1.0"
}
this.versionsMap: Map(1) {
'.' => Version {
major: 0,
minor: 2,
patch: 0,
preRelease: undefined,
build: undefined
}
}
parsed - updated: { '.': '0.2.0' }
<<<<<<<<<<<<<<<<<<<< END LOGS @ class ReleasePleaseManifest.updateContent <<<<<<<<<<<<<<<<<<<<and the secondly pushed release please manifest update corresponds with the logs:
>>>>>>>>>>>>>>>>>>>> START LOGS @ class ReleasePleaseManifest.updateContent >>>>>>>>>>>>>>>>>>>>
content: {
".": "0.1.0"
}
this.versionsMap: Map(0) {}
parsed - updated: { '.': '0.1.0' }
<<<<<<<<<<<<<<<<<<<< END LOGS @ class ReleasePleaseManifest.updateContent <<<<<<<<<<<<<<<<<<<<And seems like those updates are performed independently. This means that each updater modifies original source file at the remote repository, not modifies whatever it is modified by the previous updater. We can notice, in the end, 2 updaters exist at the end of the entire class Manifest.buildPullRequests run.
# ...
newUpdates - release-please-manifest: [
{
path: '.release-please-manifest.json',
createIfMissing: false,
updater: ReleasePleaseManifest { version: [Version], versionsMap: [Map] }
},
{
path: '.release-please-manifest.json',
createIfMissing: false,
updater: ReleasePleaseManifest {
version: undefined,
versionsMap: Map(0) {}
}
}
]
newUpdates - release-please-manifest - updater [
ReleasePleaseManifest {
version: Version {
major: 0,
minor: 2,
patch: 0,
preRelease: undefined,
build: undefined
},
versionsMap: Map(1) { '.' => [Version] }
},
ReleasePleaseManifest { version: undefined, versionsMap: Map(0) {} }
]
<<<<<<<<<<<<<<<<<<<< END LOGS @ class WorkspacePlugin.run @ const newUpdates <<<<<<<<<<<<<<<<<<<<In sum, it appears that secondly pushed release please manifest update by abstract class WorkspacePlugin
overrides the firstly pushed update which correctly bumps the version.
Possible solution I found after the digging process above, I successfully bumped the version by changing:
// https://github1s.com/googleapis/release-please/blob/main/src/plugins/workspace.ts
export abstract class WorkspacePlugin<T> extends ManifestPlugin {
/* ... */
async run(/* ... */): /* ... */ {
/* ... */
const newUpdates = newCandidates[0].pullRequest.updates;
newUpdates.push({
path: this.manifestPath,
createIfMissing: false,
updater: new ReleasePleaseManifest({
version: newCandidates[0].pullRequest.version!,
versionsMap: updatedPathVersions,
}),
});
/* ... */
}
/* ... */
}to:
// https://github1s.com/googleapis/release-please/blob/main/src/plugins/workspace.ts
export abstract class WorkspacePlugin<T> extends ManifestPlugin {
/* ... */
async run(/* ... */): /* ... */ {
/* ... */
const newUpdates = newCandidates[0].pullRequest.updates;
const releasePleaseManifestUpdates = newUpdates.filter(
(update) => update.path === this.manifestPath
);
if (releasePleaseManifestUpdates.length === 0) {
newUpdates.push({
path: this.manifestPath,
createIfMissing: false,
updater: new ReleasePleaseManifest({
version: newCandidates[0].pullRequest.version!,
versionsMap: updatedPathVersions,
}),
});
}
/* ... */
}
/* ... */
}which correctly updates .release-please-manifest.json and yields:
But I don't sure if this change stands against to the original intention of code authors, if there exists.
Please let me know if that update push made in abstract class WorkspacePlugin.run cannot be omitted conditionally and should always be pushed.
As a compare case, where the updates of .release-please-manifest.json correctly happened, I also delved into the case of [01Joseph-Hwang10/release-please-with-root-example#1].
I ran:
release-please release-pr --token="$GITHUB_TOKEN" --repo-url="https://github.com/01Joseph-Hwang10/release-please-monorepo-with-root-example" --trace --dry-runwhich yielded:
In this case, it appears that, in the end, only one class CompositeUpdater for .release-please-manifest.json exists.
So each update is applied sequentially, preserving every update pushed during the execution.
This seems to be happening because class Merge plugin merges every update pushed by previously executed functions and methods, which is, AFAIK, only used when there are multiple packages to release. (Correct me if I'm wrong)
# ...
updates - merged - release-please-manifest: [
{
path: '.release-please-manifest.json',
createIfMissing: false,
updater: CompositeUpdater { updaters: [Array] }
}
]
updates - merged - release-please-manifest - updater: [
CompositeUpdater {
updaters: [ [CompositeUpdater], [ReleasePleaseManifest] ]
}
]
<<<<<<<<<<<<<<<<<<<< END LOGS @ class Merge.run @ const updates <<<<<<<<<<<<<<<<<<<<
bukowa
commented
4 months ago Happens to me too with node-workspace plugin when root package.json has more than 2 workspaces and one is not defined in packages section of release-please-config.json (dependency bump).
After debugging I can confirm it overwrites first update like @01Joseph-Hwang10 found out.
bukowa
commented
4 months ago I fixed this in above commit, take a look at config key separate-pull-requests - schema says it's false by default - I wonder why when I explicitly set this to false it starts to work all of a sudden? ^^ No idea but trying to find out.
Djiit
commented
3 months ago Hey team, happening to us as well on a bare nodejs mono repo. Anything we can do on our side to help repro or fix the issue? Thanks
wheresrhys
commented
3 months ago This worked for me https://github.com/wheresrhys/fetch-mock/pull/706
We use
release-pleaseto create releases for the Puppeteer project. The current structure is a mono repo so we use a configuration file to express this. This works well for the 3 packages that depend on each otherpuppeteer,puppeteer-core, and@puppeteer/browsers. But for the last one@puppeteer/ng-schematicswe are seeing issue that the release does not update.release-please-manifest.json.This can be observed on the latest PR - https://github.com/puppeteer/puppeteer/pull/11543. If we do merge this PR without updating the manifest manually
release-pleasewill open a new one as the version differs with the manifest file. Observed: Initial - https://github.com/puppeteer/puppeteer/pull/11496 A second one - https://github.com/puppeteer/puppeteer/pull/11508The issue is not consistent. It doesn't fail all the time as you can see from https://github.com/puppeteer/puppeteer/pull/11488
Environment details
Steps to reproduce