Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fails to massage pnpm release notes #16921

Closed
ylemkimon opened this issue Aug 3, 2022 · 4 comments · Fixed by #17466
Closed

Fails to massage pnpm release notes #16921

ylemkimon opened this issue Aug 3, 2022 · 4 comments · Fixed by #17466
Assignees
Labels
core:changelogs Related to changelogs/release notes fetching priority-2-high Bugs impacting wide number of users or very important features status:blocked Issue is blocked by another issue or external requirement type:bug Bug fix of existing functionality

Comments

@ylemkimon
Copy link
Contributor

ylemkimon commented Aug 3, 2022

How are you running Renovate?

Mend Renovate hosted app on github.com

If you're self-hosting Renovate, tell us what version of Renovate you run.

No response

Please select which platform you are using if self-hosting.

No response

If you're self-hosting Renovate, tell us what version of the platform you run.

No response

Was this something which used to work for you, and then stopped?

It used to work, and then stopped

Describe the bug

Renovate fails to massage issue links in the What's Changed section of pnpm release notes, leading to autolinks, e.g., renovatebot/docker-buildpack#338.

It seems to be caused by an HTML table; the raw release note is as follows:

Relevant debug logs

Raw Release Note
## Minor Changes

-   A new setting supported: `prefer-symlinked-executables`. When `true`, pnpm will create symlinks to executables in
    `node_modules/.bin` instead of command shims (but on POSIX systems only).

    This setting is `true` by default when `node-linker` is set to `hoisted`.

    Related issue: [#4782](https://github.com/pnpm/pnpm/issues/4782).

-   When `lockfile-include-tarball-url` is set to `true`, every entry in `pnpm-lock.yaml` will contain the full URL to the package's tarball [#5054](https://github.com/pnpm/pnpm/pull/5054).

## Patch Changes

-   `pnpm deploy` should include all dependencies by default [#5035](https://github.com/pnpm/pnpm/issues/5035).
-   Don't print warnings about file verifications. Just print info messages instead.
-   `pnpm publish --help` should print the `--recursive` and `--filter` options [#5019](https://github.com/pnpm/pnpm/issues/5019).
-   It should be possible to run exec/run/dlx with the `--use-node-version` option.
-   `pnpm deploy` should not modify the lockfile [#5071](https://github.com/pnpm/pnpm/issues/5071)
-   `pnpm deploy` should not fail in CI [#5071](https://github.com/pnpm/pnpm/issues/5071)
-   When `auto-install-peers` is set to `true`, automatically install direct peer dependencies [#5028](https://github.com/pnpm/pnpm/pull/5067).

    So if your project the next manifest:

    ```json
    {
      "dependencies": {
        "lodash": "^4.17.21"
      },
      "peerDependencies": {
        "react": "^18.2.0"
      }
    }
    ```

    pnpm will install both lodash and react as a regular dependencies.


## Our Gold Sponsors

<table>
  <tbody>
    <tr>
      <td align="center" valign="middle">
        <a href="https://bit.dev/?utm_source=pnpm&utm_medium=release_notes" target="_blank"><img src="https://raw.githubusercontent.com/pnpm/pnpm.github.io/main/static/img/users/bit.svg" width="80"></a>
      </td>
      <td align="center" valign="middle">
        <a href="https://nhost.io/?utm_source=pnpm&utm_medium=release_notes" target="_blank"><img src="https://raw.githubusercontent.com/pnpm/pnpm.github.io/main/static/img/users/nhost.svg" width="180"></a>
      </td>
      <td align="center" valign="middle">
        <a href="https://novu.co/?utm_source=pnpm&utm_medium=release_notes" target="_blank"><img src="https://raw.githubusercontent.com/pnpm/pnpm.github.io/main/static/img/users/novu.svg" width="180"></a>
      </td>
    </tr>
  </tbody>
</table>

## Our Silver Sponsors

<table>
  <tbody>
    <tr>
      <td align="center" valign="middle">
        <a href="https://prisma.io/?utm_source=pnpm&utm_medium=release_notes" target="_blank"><img src="https://raw.githubusercontent.com/pnpm/pnpm.github.io/main/static/img/users/prisma.svg" width="180"></a>
      </td>
      <td align="center" valign="middle">
        <a href="https://leniolabs.com/?utm_source=pnpm&utm_medium=release_notes" target="_blank"><img src="https://raw.githubusercontent.com/pnpm/pnpm.github.io/main/static/img/users/leniolabs.jpg" width="80"></a>
      </td>
      <td align="center" valign="middle">
        <a href="https://vercel.com/?utm_source=pnpm&utm_medium=release_notes" target="_blank"><img src="https://raw.githubusercontent.com/pnpm/pnpm.github.io/main/static/img/users/vercel.svg" width="180"></a>
      </td>
      <td align="center" valign="middle">
        <a href="https://www.takeshape.io/?utm_source=pnpm&utm_medium=release_notes" target="_blank"><img src="https://raw.githubusercontent.com/pnpm/pnpm.github.io/main/static/img/users/takeshape.svg" width="280"></a>
      </td>
    </tr>
  </tbody>
</table>

## What's Changed
* pnpm rebuild accepts --store-dir by @chengcyber in https://github.com/pnpm/pnpm/pull/5036
* fix(deploy): include all deps by default by @zkochan in https://github.com/pnpm/pnpm/pull/5040
* chore(deps): upgrade nock to v13 by @mcmxcdev in https://github.com/pnpm/pnpm/pull/5043
* fix: log more info on HTTP error by @zkochan in https://github.com/pnpm/pnpm/pull/4917
* fix: document the -r option by @zkochan in https://github.com/pnpm/pnpm/pull/5044
* chore(deps): upgrade sinon to v14 by @mcmxcdev in https://github.com/pnpm/pnpm/pull/5045
* fix(audit): add authentication to pnpm-audit by @sled in https://github.com/pnpm/pnpm/pull/5053
* feat: prefer-symlinked-executables by @zkochan in https://github.com/pnpm/pnpm/pull/5048
* chore: update pnpm-workspace.yaml by @ayu14214 in https://github.com/pnpm/pnpm/pull/5060
* feat: add `lockfile-include-tarball-url` option by @MBelniak in https://github.com/pnpm/pnpm/pull/5054
* fix: auto install root peer deps when auto-install-peers=true by @zkochan in https://github.com/pnpm/pnpm/pull/5067
* fix(deploy): don't modify the lockfile and fail in CI by @zkochan in https://github.com/pnpm/pnpm/pull/5074

## New Contributors
* @mcmxcdev made their first contribution in https://github.com/pnpm/pnpm/pull/5043
* @sled made their first contribution in https://github.com/pnpm/pnpm/pull/5053
* @ayu14214 made their first contribution in https://github.com/pnpm/pnpm/pull/5060
* @MBelniak made their first contribution in https://github.com/pnpm/pnpm/pull/5054

**Full Changelog**: https://github.com/pnpm/pnpm/compare/v7.5.2...v7.6.0

Have you created a minimal reproduction repository?

No reproduction, but I have linked to a public repo where it occurs

@ylemkimon ylemkimon added priority-5-triage status:requirements Full requirements are not yet known, so implementation should not be started type:bug Bug fix of existing functionality labels Aug 3, 2022
@rarkins rarkins added core:changelogs Related to changelogs/release notes fetching priority-2-high Bugs impacting wide number of users or very important features auto:reproduction A minimal reproduction is necessary to proceed and removed priority-5-triage labels Aug 3, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Aug 3, 2022

Hi there,

Get your issue fixed faster by creating a minimal reproduction. This means a repository dedicated to reproducing this issue with the minimal dependencies and config possible.

Before we start working on your issue we need to know exactly what's causing the current behavior. A minimal reproduction helps us with this.

To get started, please read our guide on creating a minimal reproduction.

We may close the issue if you, or someone else, haven't created a minimal reproduction within two weeks. If you need more time, or are stuck, please ask for help or more time in a comment.

Good luck,

The Renovate team

@HonkingGoose
Copy link
Collaborator

Here's our reproduction! 😉

renovate-reproductions/16921 on GitHub

@HonkingGoose HonkingGoose added reproduction:confirmed and removed auto:reproduction A minimal reproduction is necessary to proceed labels Aug 11, 2022
@PhilipAbed PhilipAbed assigned PhilipAbed and unassigned PhilipAbed Aug 15, 2022
@MaronHatoum MaronHatoum self-assigned this Aug 16, 2022
@MaronHatoum
Copy link
Contributor

After investigating and debugging this issue, seems like there are some '\n' missing when getting release notes by 'getChangelogs()'.
I added a new regex in sanitizeMarkdown() to detect the missing '\n' and add them

  res = res.replace(regEx(/(?<before>[^\n]\n)(?<title>####.*)/g), '$<before>\n$<title>');

as a result of this change, I found another issue in:

function collectLinkPosition(input: string, matches: UrlMatch[]): Plugin {
const transformer = (tree: Content): void => {
const startOffset: number = tree.position?.start.offset ?? 0;
const endOffset: number = tree.position?.end.offset ?? 0;
if (tree.type === 'link') {
const substr = input.slice(startOffset, endOffset);
const url: string = tree.url;
const offset: number = startOffset + substr.lastIndexOf(url);
if (urlRegex.test(url)) {
matches.push({
start: offset,
end: offset + url.length,
replaceTo: massageLink(url),
});
}
} else if (tree.type === 'text') {
const globalUrlReg = new RegExp(urlRegex, 'gi');
const urlMatches = [...tree.value.matchAll(globalUrlReg)];
for (const match of urlMatches) {
const [url] = match;
const start = startOffset + (match.index ?? 0);
const end = start + url.length;
const newUrl = massageLink(url);
matches.push({ start, end, replaceTo: `[${url}](${newUrl})` });
}
} else if (hasKey('children', tree)) {
tree.children.forEach((child: Content) => {
transformer(child);
});
}
};
return () => transformer as Transformer;
}

this function should return an array that contains all the links in prbody, every object in this array contains from which character the link starts, ends, and what should the link be replaced with.

so after debugging I found that the start and end numbers that the function returns for each link are incorrect and what's changed will not look as expected.
result:
MaronHatoum/16921#24

The problem is when running

const urlMatches = [...tree.value.matchAll(globalUrlReg)];

the tree.value will be : pnpm rebuild accepts --store-dir by @​chengcyber in https://github.com/pnpm/pnpm/pull/5036

image

but if I copy this value and check it in regex101 I see that there are some characters does not appear in tree.value(in this case "U+200B")

image

so when getting the startOffset, there are 6 characters missing and the number will be startOffset-6.

@MaronHatoum MaronHatoum added the status:blocked Issue is blocked by another issue or external requirement label Aug 25, 2022
@MaronHatoum
Copy link
Contributor

blocked by: #17404

@HonkingGoose HonkingGoose removed the status:requirements Full requirements are not yet known, so implementation should not be started label Aug 25, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
core:changelogs Related to changelogs/release notes fetching priority-2-high Bugs impacting wide number of users or very important features status:blocked Issue is blocked by another issue or external requirement type:bug Bug fix of existing functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants