fix(windows): fixed issues preventing execution from windows #2672

webstech · 2023-01-15T05:52:56Z

Updates to support running on Windows post the esm conversion.

fix: Detect git version on windows
git does not fully match semver, especially on the windows version. Using loose checking in findVersions is able to match the version.
fix: use file:// url loading plugins
The file:// url is required for the win32platform but supported on all platforms.

git does not fully match semver, especially on the windows version. Using loose checking in findVersions is able to match the version. Signed-off-by: Chris. Webster <chris@webstech.net>

webstech · 2023-01-15T06:31:06Z

The tests fail on windows (mostly - have not checked them all yet) due to testdouble #491. The issue has been open for awhile.

webstech · 2023-01-15T07:05:12Z

@travi Can you approve the workflow run? I am researching how the tests fail on windows unrelated to these changes. That should not stop this PR from moving forward.

travi · 2023-01-15T17:45:34Z

just to confirm, do you mean v20+ in the PR title?

webstech · 2023-01-15T19:10:44Z

Looks like one of the test jobs is hung up somewhere.

travi

thanks a lot for the contribution. would it be possible to include some tests that simulate the situations that require these changes?

travi · 2023-01-16T06:14:57Z

lib/plugins/utils.js

+    const file = resolveFrom.silent(basePath, name) || resolveFrom(cwd, name);
+    // See https://github.com/mysticatea/eslint-plugin-node/issues/250
+    // eslint-disable-next-line node/no-unsupported-features/es-syntax
+    name = (await import(`${ file[0] != '.' ? "file://" : "" }${file}`)).default;


so i dont make incorrect assumptions, could you highlight the details of this change?

This change uses the file:// url schema for import if the file path is not relative (starts with '.'). Node doc indicates this works for import on all platforms. This is a cheap test instead of using path.isAbsolute. If plugins can be referenced using http: schema, this may need to be changed.

This is a cheap test instead of using path.isAbsolute.

the benefit of path.isAbsolute is that it is self-documenting about why the check is in place. i think i actually prefer that approach here for that reason. any reason not to go that route?

any reason not to go that route?

After looking at the code some more, it appears file is always absolute and no check is needed. I have removed the check so file:// is always prepended. Tests under Windows ran fine.

this question did go through my mind, but i dismissed it thinking that loading by package name might be the other case. assuming we can get passing builds on this PR, i'll get it this merged.

travi · 2023-01-16T06:16:02Z

bin/semantic-release.js

@@ -25,7 +25,7 @@ See https://github.com/semantic-release/semantic-release/blob/master/docs/suppor

 execa("git", ["--version"])
  .then(({ stdout }) => {
-    const gitVersion = findVersions(stdout)[0];
+    const gitVersion = findVersions(stdout, { loose: true })[0];


so we capture details that we can look back to in the future, could you mention the details that lead to this change?

The output of git -v on Windows at my current install is: git version 2.39.0.windows.2. This is not fully semver compliant. Issue #git-for-windows/git#4086 was opened (and closed) recently regarding this (tl;dr it is not going to change). It appears the semver package works and the find-versions package does not. The loose option extracts the version info from the string correctly. The non-Windows versions could have similar issues in the future that this will hopefully avoid. find-versions could be updated but a rejection would not be unexpected.

webstech · 2023-01-16T07:18:44Z

would it be possible to include some tests that simulate the situations that require these changes?

Simply running on Windows tests these changes. Running elsewhere shows they do no harm. Not sure how the version test can be simulated.

b12k · 2023-01-18T20:46:54Z

Looking forward for this PR to be merged soon.
I'm facing the same issues with git versions and plugins import.

webstech · 2023-01-19T07:46:40Z

Tidying up loose ends. This is in support of issue #2658. The issue will track related changes.

webstech · 2023-01-19T08:06:43Z

Running elsewhere shows they do no harm.

Is there any reason to hold this back when the tests run on non-Windows? Not sure what tests I could do for this. As noted in the related issue, patches will be coming to allow Windows as part of the CI tests.

travi · 2023-01-20T21:59:02Z

Simply running on Windows tests these changes. Running elsewhere shows they do no harm. Not sure how the version test can be simulated.

sorry for the delay responding to this question. i was hopeful that we'd be able to add tests that were specific to the new code paths based on these scenarios, but i finally had a chance to review our existing tests and it looks like the only points to observe those particular behaviors are currently at the system level. so, without executing the tests on windows where the underlying behavior is actually different, i dont think we could actually test these things w/o bigger changes to our tests.

based on that, lets handle the path.isAbsolute point that i mentioned above and then i think we're ready to get this merged and released.

The `file://` url is required for the `win32`platform but supported on all platforms. Signed-off-by: Chris. Webster <chris@webstech.net>

travi · 2023-01-24T02:37:19Z

i'm going to move ahead with merging this even though we havent seen all of the checks pass yet. the tests are not existing correctly in v19 after timing out, which appears to be happening pretty consistently and i have tried restarting several times. those problems are not unique to this branch since they also exist on the mainline and in other PRs.

github-actions · 2023-01-24T02:39:39Z

🎉 This PR is included in version 20.0.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

webstech · 2023-01-24T07:22:40Z

i'm going to move ahead with merging this

Thank you.

travi · 2023-01-24T13:34:15Z

i'm going to move ahead with merging this

Thank you.

Thank you for this contribution. It was a huge help getting us past these issues.

github-actions · 2023-01-27T22:11:12Z

🎉 This PR is included in version 21.0.0-beta.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

webstech changed the title ~~Windows support for v19+~~ Windows support for v20+ Jan 15, 2023

travi reviewed Jan 16, 2023

View reviewed changes

webstech mentioned this pull request Jan 19, 2023

Semantic Release 20 can't detect git installed in Github Action Windows #2658

Closed

travi approved these changes Jan 21, 2023

View reviewed changes

Merge branch 'master' into windows-git

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

3eec0bd

travi changed the title ~~Windows support for v20+~~ fix(windows): fixed issues preventing execution from windows Jan 24, 2023

travi merged commit 5df624c into semantic-release:master Jan 24, 2023

github-actions bot added the released label Jan 24, 2023

webstech deleted the windows-git branch January 24, 2023 07:22

github-actions bot added the released on @beta label Jan 27, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(windows): fixed issues preventing execution from windows #2672

fix(windows): fixed issues preventing execution from windows #2672

webstech commented Jan 15, 2023 •

edited

Loading

webstech commented Jan 15, 2023

webstech commented Jan 15, 2023

travi commented Jan 15, 2023

webstech commented Jan 15, 2023

travi left a comment

travi Jan 16, 2023

webstech Jan 16, 2023

travi Jan 20, 2023

webstech Jan 21, 2023

travi Jan 21, 2023

travi Jan 16, 2023

webstech Jan 16, 2023

webstech commented Jan 16, 2023

b12k commented Jan 18, 2023

webstech commented Jan 19, 2023

webstech commented Jan 19, 2023

travi commented Jan 20, 2023

travi commented Jan 24, 2023

github-actions bot commented Jan 24, 2023

webstech commented Jan 24, 2023

travi commented Jan 24, 2023

github-actions bot commented Jan 27, 2023

fix(windows): fixed issues preventing execution from windows #2672

fix(windows): fixed issues preventing execution from windows #2672

Conversation

webstech commented Jan 15, 2023 • edited Loading

webstech commented Jan 15, 2023

webstech commented Jan 15, 2023

travi commented Jan 15, 2023

webstech commented Jan 15, 2023

travi left a comment

Choose a reason for hiding this comment

travi Jan 16, 2023

Choose a reason for hiding this comment

webstech Jan 16, 2023

Choose a reason for hiding this comment

travi Jan 20, 2023

Choose a reason for hiding this comment

webstech Jan 21, 2023

Choose a reason for hiding this comment

travi Jan 21, 2023

Choose a reason for hiding this comment

travi Jan 16, 2023

Choose a reason for hiding this comment

webstech Jan 16, 2023

Choose a reason for hiding this comment

webstech commented Jan 16, 2023

b12k commented Jan 18, 2023

webstech commented Jan 19, 2023

webstech commented Jan 19, 2023

travi commented Jan 20, 2023

travi commented Jan 24, 2023

github-actions bot commented Jan 24, 2023

webstech commented Jan 24, 2023

travi commented Jan 24, 2023

github-actions bot commented Jan 27, 2023

webstech commented Jan 15, 2023 •

edited

Loading