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

Shim IO#pread when not supported #52

Merged
merged 2 commits into from
Aug 8, 2023

Conversation

casperisfine
Copy link

@casperisfine casperisfine commented Aug 4, 2023

Followup: #50

Typically on Windows.

@casperisfine
Copy link
Author

Ah, on TruffleRuby as well it's missing.

@casperisfine
Copy link
Author

cc @SamSaffron, really sorry for not catching this earlier.

@casperisfine
Copy link
Author

Hum, I'm not too sure what is going on on windows :/

@casperisfine
Copy link
Author

Ok, so it seems that on Windows the index is offsettted because of \r:

[:info, "                                     base64          \r\nmseq        application/vnd.mseq                "]
[:info, "                                          base64          \r\nmsl         application/vnd.Mobius.MSL     "]
[:info, "                                            base64          \r\nmts         model/vnd.mts                "]
[:info, "                                             base64          \r\nmus         application/vnd.musician    "]
[:info, "                                              base64          \r\nmusicxml    application/vnd.recordare.m"]

I'll try to figure out a solution, it's just very painful to not be able to figure this out locally :/

# We must open the file in binary mode
# otherwise Ruby's automatic line terminator
# translation will skew the row size
@file = ::File.open(filename, 'rb')
Copy link
Author

Choose a reason for hiding this comment

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

Ok, so it's because for IOs in text mode, Ruby automatically replace \r\n by \n, so the row_length end up being off by one.

@casperisfine
Copy link
Author

Hum, there is one failure left on Windows that I can't make much sense of:

  1) Failure:
MiniMimeTest#test_full_parity_with_mime_types [D:/a/mini_mime/mini_mime/test/mini_mime_test.rb:76]:
Expected: "application/x-cu-seeme"
  Actual: "application/cu-seeme"

Typically on Windows.
@casperisfine
Copy link
Author

Nevermind, that test compare results with the mime-types gem, so I suspect windows machines have a different mapping for some types.

@SamSaffron
Copy link
Member

No worries, can you add a special case in the test for windows then? I don't think there is a mega rush here, Ruby on Windows is really really painful

@ahorek
Copy link

ahorek commented Aug 6, 2023

Nevermind, that test compare results with the mime-types gem, so I suspect windows machines have a different mapping for some types.

this is most likely because Ruby doesn't have a stable sort, see:
mime-types/ruby-mime-types#148 and https://bugs.ruby-lang.org/issues/1089

mime_x = 1
mime_y = 1
mime_x <> mime_y

in this case, if you compare 2 mime types with the same priority, the result will be mime_x on Linux and usually mime_y on Windows (yes, it's silly). Since mime_x has the same priority as mime_y, both results are actually correct.

I would say, it's a bug in mime-types's implementation and it's safe to ignore it...

@SamSaffron
Copy link
Member

sounds good, I guess we just add a special case into the spec?

@casperisfine casperisfine force-pushed the shim-pread branch 2 times, most recently from 68e93ec to 858f145 Compare August 7, 2023 07:36
@casperisfine
Copy link
Author

Ok, so here's the list of differences on Windows:

Expected ".cu" to return "application/x-cu-seeme", got: "application/cu-seeme"
Expected ".ecma" to return "text/ecmascript", got: "application/ecmascript"
Expected ".es" to return "text/ecmascript", got: "application/ecmascript"
Expected ".jar" to return "application/x-java-archive", got: "application/java-archive"
Expected ".ser" to return "application/x-java-serialized-object", got: "application/java-serialized-object"
Expected ".mp4" to return "audio/mp4", got: "application/mp4"
Expected ".mpg4" to return "audio/mp4", got: "application/mp4"
Expected ".doc" to return "text/plain", got: "application/msword"
Expected ".pgp" to return "application/pgp-encrypted", got: "application/octet-stream"
Expected ".gpg" to return "application/pgp-encrypted", got: "application/octet-stream"
Expected ".ai" to return "application/postscript", got: "application/pdf"
Expected ".asc" to return "text/plain", got: "application/pgp-signature"
Expected ".rtf" to return "text/rtf", got: "application/rtf"
Expected ".spp" to return "application/vnd.sealed.ppt", got: "application/scvp-vp-response"
Expected ".sgml" to return "text/sgml", got: "application/sgml"
Expected ".curl" to return "text/vnd.curl", got: "application/vnd.curl"
Expected ".odc" to return "application/vnd.oasis.opendocument.chart-template", got: "application/vnd.oasis.opendocument.chart"
Expected ".odf" to return "application/vnd.oasis.opendocument.formula-template", got: "application/vnd.oasis.opendocument.formula"
Expected ".odi" to return "application/vnd.oasis.opendocument.image-template", got: "application/vnd.oasis.opendocument.image"
Expected ".bdm" to return "video/MP2T", got: "application/vnd.syncml.dm+wbxml"
Expected ".dcr" to return "image/x-kodak-dcr", got: "application/x-director"
Expected ".exe" to return "application/x-msdos-program", got: "application/x-ms-dos-executable"
Expected ".wmz" to return "application/x-msmetafile", got: "application/x-ms-wmz"
Expected ".cmd" to return "application/x-msdownload", got: "application/x-msdos-program"
Expected ".bat" to return "application/x-msdownload", got: "application/x-msdos-program"
Expected ".com" to return "application/x-msdownload", got: "application/x-msdos-program"
Expected ".reg" to return "application/x-msdownload", got: "application/x-msdos-program"
Expected ".ps1" to return "application/x-msdownload", got: "application/x-msdos-program"
Expected ".vbs" to return "application/x-msdownload", got: "application/x-msdos-program"
Expected ".pm" to return "application/x-perl", got: "application/x-pagemaker"
Expected ".xml" to return "text/xml", got: "application/xml"
Expected ".dtd" to return "text/xml", got: "application/xml-dtd"
Expected ".kar" to return "audio/x-midi", got: "audio/midi"
Expected ".mid" to return "audio/x-midi", got: "audio/midi"
Expected ".midi" to return "audio/x-midi", got: "audio/midi"
Expected ".m4a" to return "audio/MP4A-LATM", got: "audio/mp4"
Expected ".mp2" to return "video/mpeg", got: "audio/mpeg"
Expected ".ogg" to return "video/ogg", got: "audio/ogg"
Expected ".wav" to return "audio/x-wav", got: "audio/wav"
Expected ".webm" to return "video/webm", got: "audio/webm"
Expected ".wmv" to return "video/x-ms-wmv", got: "audio/x-ms-wmv"
Expected ".ra" to return "audio/x-realaudio", got: "audio/x-pn-realaudio"
Expected ".hif" to return "image/heic-sequence", got: "image/heic"
Expected ".sub" to return "text/vnd.dvb.subtitle", got: "image/vnd.dvb.subtitle"
Expected ".xbm" to return "image/x-xbm", got: "image/x-xbitmap"
Expected ".mts" to return "video/MP2T", got: "model/vnd.mts"
Expected ".rst" to return "text/prs.fallenstein.rst", got: "text/plain"

@casperisfine
Copy link
Author

Ok, I added a list of windows differences.

@SamSaffron
Copy link
Member

looks good to me! Thanks @casperisfine

@SamSaffron SamSaffron merged commit d7df855 into discourse:main Aug 8, 2023
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants