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

Fix missing executable extensions in tool links #18

Merged
merged 9 commits into from
Apr 24, 2024

Conversation

filiptibell
Copy link
Collaborator

@filiptibell filiptibell commented Apr 24, 2024

This PR fixes #13 by properly including the OS executable extension when creating tool links.

Also included in this PR:

  • Automatically fixing existing invalid links (ones missing an executable extension) when running rokit self-install and / or rokit install
  • A loud, user-facing warning message for invalid executable files being written, to prevent this from happening in the future, or at least make it easier to diagnose
  • Switches to a single "source of truth" internally for discovering existing tool links

Copy link
Member

@kennethloeffler kennethloeffler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't check that recreating the tool links during rokit self-install works because something else is going wrong when I run rokit self-install over an existing installation:
image

But everything else (besides the loud warning) is working well:

lib/util/fs.rs Outdated Show resolved Hide resolved
lib/storage/tool_storage.rs Outdated Show resolved Hide resolved
Copy link
Member

@kennethloeffler kennethloeffler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I'll investigate why using self-install over my existing installation didn't work and open an issue if needed later

(new release? 👀)

@filiptibell filiptibell merged commit b4377c9 into main Apr 24, 2024
10 checks passed
@filiptibell
Copy link
Collaborator Author

LGTM, I'll investigate why using self-install over my existing installation didn't work and open an issue if needed later

(new release? 👀)

Sounds good to me! Will update the changelog and run a new release 🫡

@filiptibell filiptibell deleted the fix-executable-suffixes branch April 24, 2024 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rokit does not add the exe extension to tool links on Windows, making installed tools unusable
2 participants