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

Pre-release changes still not working [Bug] #158

Closed
nerdoza opened this issue Apr 18, 2022 · 19 comments · Fixed by #257
Closed

Pre-release changes still not working [Bug] #158

nerdoza opened this issue Apr 18, 2022 · 19 comments · Fixed by #257
Labels
status: pinned Should not be labeled as stale type: bug Verified problems that need to be worked on

Comments

@nerdoza
Copy link

nerdoza commented Apr 18, 2022

Describe the bug
Using the diff-search: true option, pre-release changes are not properly detected.

To Reproduce
Moving from "version": "2.5.0-beta.0" to "version": "2.5.0-beta.1" in the package.json file results in the following output:

Run EndBug/version-check@v[2](https://github.com/KeiserCorp/Keiser.A500.Display.Main/runs/6068099586?check_suite_focus=true#step:4:2)
  with:
    diff-search: true
    token: ***
    file-name: package.json
    github-api-url: https://api.github.com
Searching for version update...
Searching in 1 commit...
  Package file name: "package.json"
  Package file URL: undefined
  Version assumptions: undefined
No standard npm version commit found, switching to diff search (this could take more time...)
Checking the diffs of 1 commit...
  - 97615[4](https://github.com/KeiserCorp/Keiser.A500.Display.Main/runs/6068099586?check_suite_focus=true#step:4:4)7: added version doesn't match current one (added: "2.[5](https://github.com/KeiserCorp/Keiser.A500.Display.Main/runs/6068099586?check_suite_focus=true#step:4:5).0-beta"; current: "2.5.0-beta.1")
No matching commit found.

Outputs:
  changed: false

Expected behavior
Should result in changed: true.

Additional details
While digging for this issue I came to lines 394-396 of the main.ts file where I suspect the issue resides, though I don't know the output of the included package to determine if this is indeed the issue.

version-check/src/main.ts

Lines 394 to 396 in 0a49022

function parseVersionLine(str: string) {
return (str.split('"') || []).map((s) => matchVersion(s)).find((e) => !!e)
}

@nerdoza nerdoza added the status: pending More info is needed before deciding what to do label Apr 18, 2022
@EndBug EndBug added type: bug Verified problems that need to be worked on status: pinned Should not be labeled as stale and removed status: pending More info is needed before deciding what to do labels Apr 18, 2022
@SwabianCoder
Copy link

SwabianCoder commented Jul 23, 2022

I wanted to use your GitHub action in my GitHub worflow. Unfortunately I'm facing the same issue.

@SwabianCoder
Copy link

As a workaround I use pysemver CLI now. Works great with pre-release versions as well.

@SwabianCoder
Copy link

SwabianCoder commented Jul 24, 2022

Describe the bug

Using the diff-search: true option, pre-release changes are not properly detected.

To Reproduce

Moving from "version": "2.5.0-beta.0" to "version": "2.5.0-beta.1" in the package.json file results in the following output:


Run EndBug/version-check@v[2](https://github.com/KeiserCorp/Keiser.A500.Display.Main/runs/6068099586?check_suite_focus=true#step:4:2)

  with:

    diff-search: true

    token: ***

    file-name: package.json

    github-api-url: https://api.github.com

Searching for version update...

Searching in 1 commit...

  Package file name: "package.json"

  Package file URL: undefined

  Version assumptions: undefined

No standard npm version commit found, switching to diff search (this could take more time...)

Checking the diffs of 1 commit...

  - 97615[4](https://github.com/KeiserCorp/Keiser.A500.Display.Main/runs/6068099586?check_suite_focus=true#step:4:4)7: added version doesn't match current one (added: "2.[5](https://github.com/KeiserCorp/Keiser.A500.Display.Main/runs/6068099586?check_suite_focus=true#step:4:5).0-beta"; current: "2.5.0-beta.1")

No matching commit found.



Outputs:

  changed: false

Expected behavior

Should result in changed: true.

Additional details

While digging for this issue I came to lines 394-396 of the main.ts file where I suspect the issue resides, though I don't know the output of the included package to determine if this is indeed the issue.

version-check/src/main.ts

Lines 394 to 396 in 0a49022

function parseVersionLine(str: string) {
return (str.split('"') || []).map((s) => matchVersion(s)).find((e) => !!e)
}

Also 1.0.0-beta.0 should be detected to be smaller than 1.0.0-rc.0. Current behavior is that both versions are detected to be equal.

@HarelM
Copy link

HarelM commented Nov 23, 2023

I have the same issue.
I'm using this action and just released 4.0.0-pre.1 and I got this log:

Run EndBug/version-check@v2
  with:
    diff-search: false
    file-name: package.json
    token: ***
    github-api-url: https://api.github.com
Searching for version update...
Searching in 1 commit...
  Package file name: "package.json"
  Package file URL: undefined
  Version assumptions: undefined
No standard npm version commit found, switching to diff search (this could take more time...)
Checking the diffs of 1 commit...
  - 08[4](https://github.com/maplibre/maplibre-gl-js/actions/runs/6968190440/job/18961591361#step:4:4)ef[5](https://github.com/maplibre/maplibre-gl-js/actions/runs/6968190440/job/18961591361#step:4:5)[6](https://github.com/maplibre/maplibre-gl-js/actions/runs/6968190440/job/18961591361#step:4:6): added version doesn't match current one (added: "4.0.0"; current: "4.0.0-pre.1")
No matching commit found.
Outputs:
  changed: false

@SwabianCoder how did you solve this?

This is my workflow file:
https://github.com/maplibre/maplibre-gl-js/blob/084ef56ccd357d280ededd194370fc85181f3f43/.github/workflows/release.yml#L26

@SwabianCoder
Copy link

SwabianCoder commented Nov 24, 2023

@HarelM I use pysemver CLI to compare versions. This works great.

pysemver compare ${{ env.latestNpmPackageVersion }} ${{ env.prPackageVersion }}

@HarelM
Copy link

HarelM commented Nov 24, 2023

Can you link to a GitHub action with this?

@HarelM
Copy link

HarelM commented Nov 25, 2023

Thanks @SwabianCoder!! I have switched to a very basic comparison based on your code. See the above linked PR.

@SwabianCoder
Copy link

@HarelM But based on that comparison you can't determine whether version increased. What if someone decreased it instead?

@HarelM
Copy link

HarelM commented Nov 26, 2023

Yes, that's true, but every PR is reviewed before merged, and we have branch protection, so I hope it's good enough, worst case I'll improve it later on, I needed to release a pre release, this was my number one goal...

@kusyka911
Copy link
Contributor

kusyka911 commented Dec 18, 2023

@EndBug
Issue still exists for static-checking. Mostly because of using semver-regex.

Using it on prerelease versions like 1.0.0-beta.1 or metadata like 1.0.0+build.1 will result in 1.0.0. While semver-diff will output correct difference (prereleas, patch, build)

And you are comparing versions from remote and local package.json files after matching it with semver-regex before using semver-diff.

Lines 100-119 at src/main.ts

 if (
      (local.match(semverRegex()) || [])[0] !=
      (remote.match(semverRegex()) || [])[0]
    ) {
      output('changed', true)
      output('version', staticChecking == 'localIsNew' ? local : remote)
      output(
        'type',
        staticChecking == 'localIsNew'
          ? semverDiff(remote, local)
          : semverDiff(local, remote)
      )

      endGroup()
      info(
        `Found match for version ${
          staticChecking == 'localIsNew' ? local : remote
        }`
      )
    }

See codesandbox demo here: https://codesandbox.io/p/devbox/interesting-euclid-4wkr5g

solution proposal:

    const versionDiff = staticChecking == 'localIsNew'
      ? semverDiff(remote, local)
      : semverDiff(local, remote);

    if (versionDiff) {
      output('changed', true)
      output('version', staticChecking == 'localIsNew' ? local : remote)
      output('type', versionDiff)

      endGroup()
      info(
        `Found match for version ${
          staticChecking == 'localIsNew' ? local : remote
        }`
      )
    }

@EndBug
Copy link
Owner

EndBug commented Dec 25, 2023

This issue should be fixed by the awesome work by @kusyka911 🎉
I'll publish it right away!

@EndBug
Copy link
Owner

EndBug commented Dec 25, 2023

@all-contributors please add @bayssmekanique for bug

@EndBug

This comment was marked as resolved.

This comment was marked as resolved.

@HarelM
Copy link

HarelM commented Jan 11, 2024

Is it possible to add to the readme the possible values of type after this fix?
Is there a way for me to know that this is a pre-release using this action?

@EndBug
Copy link
Owner

EndBug commented Jan 11, 2024

@HarelM Good point, I should mention in the docs that the definition of the type is dependent on the semver-diff package

@EndBug
Copy link
Owner

EndBug commented Jan 11, 2024

@all-contributors please add @HarelM for their docs contribution

Copy link
Contributor

@EndBug

I've put up a pull request to add @HarelM! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: pinned Should not be labeled as stale type: bug Verified problems that need to be worked on
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants