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

[Bug]: Cache uses different path separators based off OS #5246

Closed
1 task done
franklygeorgy opened this issue Feb 14, 2024 · 7 comments · Fixed by #5267
Closed
1 task done

[Bug]: Cache uses different path separators based off OS #5246

franklygeorgy opened this issue Feb 14, 2024 · 7 comments · Fixed by #5267

Comments

@franklygeorgy
Copy link

franklygeorgy commented Feb 14, 2024

Kind of Issue

Runtime - command-line tools

Tool or Library

cspell

Version

8.3.2

Supporting Library

Not sure / None

OS

All of them

OS Version

Windows 10, Garuda Linux (Arch), But should affect all

Description

The caching system doesn't normalize path separators. So if you were developing on Windows and Unix/Mac you will end up recalculating the .cspellcache file each time the OS changes between Windows and Unix/Mac. Meaning that if you have a team of several developers and some of them are using Windows and some are using Unix/Mac a universal cache file is effectively useless.

Steps to Reproduce

Have universal caching setup in cspell.json

"cache": {
    "cacheFormat": "universal",
    "cacheLocation": ".cspellcache",
    "cacheStrategy": "content",
    "useCache": true
},

Run it on Windows and look at the results. The path separator will be an escaped \
Copy/Git push and pull the project onto a Unix/Mac machine
Run it again on the Unix/Mac machine and look at the results. The path separator will now be an /. All references to any paths with \ in them will be removed because they were not a valid file

Expected Behavior

The path separator would be normalized for cross platform support.

Additional Information

No response

cspell.json

{
    "version": "0.2",
    "cache": {
        "cacheFormat": "universal",
        "cacheLocation": ".cspellcache",
        "cacheStrategy": "content",
        "useCache": true
    },
    "ignorePaths": [
        "node_modules",
        "redacted",
        "*.json",
        "*.svg",
        "*.sh",
        "*.bat"
    ],
    "language": "en",
    "dictionaries": [
        "companies",
        "softwareTerms",
        "misc",
        "typescript",
        "node",
        "html",
        "css",
        "filetypes",
        "npm"
    ],
    "flagWords": [
        "redacted"
    ],
    "ignoreWords": [
        "redacted"
    ],
    "words": [
        "redacted"
    ]
}

cspell.config.yaml

No response

Example Repository

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@Jason3S
Copy link
Collaborator

Jason3S commented Feb 15, 2024

@franklygeorgy,

Thank you for reporting this. There was an issue with the underlying caching library that prevented this from working. It may have been fixed. It might be worth it to try again.

@franklygeorgy
Copy link
Author

The path separators are now the same but it still forces a rebuild. I will try and figure out where the cache is being changed as this is likely what is causing the rebuild.

@franklygeorgy
Copy link
Author

Okay, I think I see what you did. It may be working fine. Let me test a little more.

@franklygeorgy
Copy link
Author

Okay so caching is working cross platform (just not the way I would have implemented it), but something in the cache file gets changed between Windows and Unix/Mac systems that makes Git show it as modified. I am trying to work out what that change is.

@franklygeorgy
Copy link
Author

franklygeorgy commented Feb 15, 2024

So the Unix/Mac side of things still seems to be removing any path with a \ in it. Personally I would just checked if path.sep === "\\" and if so rewrite paths to use / when writing to the cache file and rewrite paths to use \ when reading from the cache file. This will make running on windows slightly slower, but assuming you don't have any other OS specific changes to the way cache files are generated this should ensure that the cache file doesn't change between systems (assuming that the underlying repository hasn't changed).

@Jason3S
Copy link
Collaborator

Jason3S commented Feb 16, 2024

@franklygeorgy,

The caching library automatically removes files from the cache that were not found. It is possible to use / for all path separators (/ is supported on Windows, but \ is not supported outside of Windows), which is why the cache works when going from Unix to Windows but not the other way.

There was code to normalize the \ to / when a file is added to the cache, but I don't remember why it isn't currently used.

I might replace the caching library because it is a bit difficult to use since it was designed for absolute paths instead of relative. It also does not support dependencies, these had to be added through the meta data option on each entry.

Jason3S added a commit that referenced this issue Feb 17, 2024
fixes: #5246

- Defaults to the universal format instead of legacy.
- Path separators are converted to `/` before being stored.
Copy link
Contributor

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants