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

ZnBufferedReadStream>>upToEnd returns nil when reading contents of an empty file #86

Closed
jbrichau opened this issue Mar 19, 2022 · 3 comments

Comments

@jbrichau
Copy link
Contributor

Case: I am reading the contents of an empty (binary) file and expect to get an empty instance of ByteArray. I now get a nil as a result instead.

This is because the ZnBufferedReadStream>>upToEnd returns nil for such a case.
The following test in Grease illustrates the issue:
GRPlatformTest>> testReadWriteEmptyFileInFolderBinary

This test fails in Pharo 10 since recently only. Probably because the latest Zinc version was included recently? I have not investigated the changes. It was tested in:

Pharo 10.0.0 Build information: Pharo-10.0.0+build.483.sha.95c615fd186704d9ae9b2d2355680145b9646774 (64 Bit)

To reproduce in Pharo 10, load Grease

Metacello new
 baseline:'Grease';
 repository: 'github://SeasideSt/Grease:master/repository';
 load:'Tests'

and execute the test testReadWriteEmptyFileInFolderBinary on GRPlatformTest

@jbrichau
Copy link
Contributor Author

A fix might be to change the following line


with:

^ result ifNil:[ self collectionSpecies new ]

@svenvc
Copy link
Owner

svenvc commented Mar 20, 2022

Hi @jbrichau

Thanks for the report: this is indeed wrong. The code is also a bit weird, but somebody will have had a reason to write it this way at one point in time I guess (maybe related to reading from stdin).

Anyway here is the fix: 66612d4

Sven

@jbrichau
Copy link
Contributor Author

thanks for the quick response @svenvc !

jbrichau added a commit to SeasideSt/Grease that referenced this issue Apr 3, 2022
@jbrichau jbrichau closed this as completed Jun 6, 2022
jbrichau added a commit to SeasideSt/Grease that referenced this issue Jun 6, 2022
jbrichau added a commit to SeasideSt/Seaside that referenced this issue Jun 6, 2022
syrel pushed a commit to feenkcom/gtoolkit that referenced this issue Jun 30, 2022
Metacello new
    baseline: 'GToolkitForPharo9';
    repository: 'github://feenkcom/gtoolkit:v0.8.1610/src';
    load

All commits (including upstream repositories) since last build:
feenkcom/Grease@23414e by Andrei Chis
Add BaselineOfGreaseForGToolkit

feenkcom/Grease@5ad8d0 by Johan Brichau
Merge pull request #138 from theseion/file-utilities-for-streaming-uploads

File utilities for streaming uploads

feenkcom/Grease@ce61d5 by Johan Brichau
Removing expected failure since svenvc/zinc#86 has been integrated into pharo 10

feenkcom/Grease@a248d0 by Max Leske
Removed #binaryWriteStreamFor:do: from Squeak

 #writeFileStreamOn:do:binary: already provides this functionality


feenkcom/Grease@3899a4 by Max Leske
Removed #binaryWriteStreamFor:do: from Pharo 7

#writeFileStreamOn:do:binary: already provides this functionality

feenkcom/Grease@c5c716 by Max Leske
Removed #binaryWriteStreamFor:do: from Pharo 10

#writeFileStreamOn:do:binary: already provides this functionality

feenkcom/Grease@47808c by Max Leske
Remove #binaryWriteStreamFor:do:

#writeFileStreamOn:do:binary: already provides this functionality

feenkcom/Grease@8b3408 by Max Leske
Add methods for streaming to Pharo 9 platform

feenkcom/Grease@4a7397 by Max Leske
Add methods used for streaming to Pharo 100 platform

feenkcom/Grease@fd607d by Max Leske
Added #binaryWriteStreamFor:do: and #newTemporaryFileReference to GRPlatform and concrete implementations to GRPharoPlatform and GRSqueakPlatform

feenkcom/Grease@269986 by Johan Brichau
CI updates: move Squeak back


feenkcom/Grease@aa57ce by Johan Brichau
Merge pull request #137 from SeasideSt/grpackage-pharo100

Move all codecs in Pharo10 to use Zinc

feenkcom/Grease@1d8dae by Johan Brichau
make thisContext test work for Squeak

feenkcom/Grease@3c4fa0 by Johan Brichau
Move all codecs in Pharo10 to use Zinc

feenkcom/Grease@74f54a by Johan Brichau
move pharo 10 out of the 'experimental' list for the CI build, move all Squeak builds to the experimental part (help wanted maintaining Squeak!)


feenkcom/Grease@c753fe by Johan Brichau
Merge pull request #136 from SeasideSt/grpackage-pharo100

fix grpackage methods for pharo100 (useless?)

feenkcom/Grease@9c3b89 by Johan Brichau
increase version number to v1.8.2

feenkcom/Grease@b2fcc5 by Johan Brichau
fix expectedFailures...

feenkcom/Grease@27f685 by Johan Brichau
Set an expected failure for testReadWriteEmptyFileInFolderBinary until fix svenvc/zinc#86 is part of the released version

feenkcom/Grease@389894 by Johan Brichau
fix grpackage methods for pharo100 (useless?)

feenkcom/Grease@3fc042 by Johan Brichau
version 1.8.1

feenkcom/Grease@160450 by vagrant
Added GsContext>>tempNamed: (for compatibility with the test in Pharo 10)


feenkcom/Grease@916778 by Johan Brichau
remove obsolete Pharo packages


feenkcom/Grease@9cfcb6 by Johan Brichau
remove package Pharo10 still did not work?

feenkcom/Grease@a97cf6 by Johan Brichau
Merge fec1cf49e3977d1d8e3ec83487281a45976654b5

feenkcom/Grease@9c769b by Johan Brichau
Seems like the package named Pharo10 was not removed previously... iceberg glitch?

feenkcom/Grease@fec1cf by Johan Brichau
Merge pull request #135 from SeasideSt/pharo100

Rename package Pharo10 to Pharo100 to avoid confusion with Pharo1

feenkcom/Grease@94f262 by Johan Brichau
rename package Pharo10 to Pharo100 to avoid confusion with Pharo1

feenkcom/Grease@db79fe by Johan Brichau
Merge pull request #134 from SeasideSt/pharo10

pharo 10

feenkcom/Grease@4a8e34 by Johan Brichau
Fix test for GRPharoGenericCodec to be skipped in Pharo10

feenkcom/Grease@7a35c9 by Johan Brichau
ping the CI for PR#134

feenkcom/Grease@2ba142 by Johan Brichau
pharo10: 
- removed codecs and texts based on TextConverter or LanguageEnvironment (deprecated  in pharo 9)
- changes sends of tempAt: by namedTempAt:

feenkcom/Grease@deed35 by Johan Brichau
remove unused class inst vars on GRGemStoneRandomProvider


feenkcom/Grease@6b9ff2 by Johan Brichau
Merge pull request #130 from SeasideSt/pharo-utf8-encoding

Use ZnUTF8Encoder instead of TextConverter in Pharo9+

feenkcom/Grease@1e872b by Johan Brichau
Added Grease-Pharo10-Core as a copy of Grease-Pharo90-Core and removed the GRDeprecatedUtf8CodecStream class>>initialize method that errored during code load due to the TextConverter not working anymore. 

feenkcom/Grease@c75170 by Johan Brichau
Use the Zn-based utf8 codec in Pharo by default

feenkcom/Grease@b06d7f by Johan Brichau
Merge 8f5255d8483bb08408d4f3c84f31fa87309108bd

feenkcom/Grease@2c53cd by Johan Brichau
- Rename GRPharoUtf8CodecStream to GRPharoDeprecatedUtf8CodecStream
- do not fall back to `utf8Decoded` message in `GRPharoUtf8Codec>>decode:`

feenkcom/Grease@8f5255 by Johan Brichau
update gemstone version (mostly to trigger build again)


feenkcom/Grease@4dbda1 by Johan Brichau
typo...


feenkcom/Grease@81d490 by Johan Brichau
include Pharo10 in the CI actions


feenkcom/Grease@44bda6 by Johan Brichau
include pharo10

feenkcom/Grease@d8b822 by Johan Brichau
Prepare for removal of TextConverter in Pharo: pull out the GRPharoZnUtf8CodecStream from the TextConverter-based code

feenkcom/magritte@13e118 by Andrei Chis
Depend on GreaseForGToolkit
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