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

Asset server content hashing may mismatch based on platform line endings #430

Closed
MaxCWhitehead opened this issue Aug 3, 2024 · 5 comments
Assignees

Comments

@MaxCWhitehead
Copy link
Collaborator

MaxCWhitehead commented Aug 3, 2024

In Jumpy, have seen CRLF on windows in assets like maps, and macOS/Linux LF, which causes mismatch in CIDs.

Wondering if we may want to add an option to FileAssetIO to force converting to linux line endings when reading asset bytes, though would need to makes sure this is only applied on text files.

Related: fishfolk/jumpy#995

@MaxCWhitehead MaxCWhitehead self-assigned this Aug 3, 2024
@zicklag
Copy link
Member

zicklag commented Aug 4, 2024

I'm wondering if maybe a better alternative is to configure git to not switch the line endings on different platforms. I'm pretty sure that's a git feature that does that.

VSCode should support the linux line endings fine even on Linux, and I think it makes the most sense that the assets shouldn't be modified by git on a per-platform basis.

@MaxCWhitehead
Copy link
Collaborator Author

Yeah I think that sounds like a decent bet - detecting text files and processing them seems dicey.

I do worry a bit about user created assets that may not be in a repo configured this way, but we can cross that bridge when we get there / provide some lint or warnings on externally loaded assets with CRLF if need be.

@MaxCWhitehead
Copy link
Collaborator Author

It looks like .gitattributes may work on reprository level on checkout to override client config. I'll test this, and take a look at our windows packaging process and make sure that ends up with right settings as well.

@zicklag
Copy link
Member

zicklag commented Aug 4, 2024

Yeah, that looked like the way you're supposed to configure it. The only thing that bugged me is that we have to add an explicit mention in that file for each kind of text file extension that we use as an asset. But that's really not that big a deal probably.

@MaxCWhitehead
Copy link
Collaborator Author

MaxCWhitehead commented Aug 5, 2024

I got away without specifying yaml or specific extensions, it seemed to resolve the issue. Folks with current CRLF checkouts will need to follow instructions on that page to update, but not worried about that. Will PR in jumpy soon.

Trying to add some project setup docs to call out this issue:
fishfolk/fishfolk.org#21

Otherwise, I think can close this, no change needed in bones.

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

No branches or pull requests

2 participants