Re: [Pharo-users] Usage of String and FileLocator to reference files in ZipArchive (and probably other places)

2018-12-14 Thread Guillermo Polito
Hi Hans,

Sorry for the late reply.

On Sun, Nov 25, 2018 at 9:30 PM Hans-Martin  wrote:

> I found a related problem which indicates that the depreciation of
> StandardFileStream in favor of File is incomplete and probably premature:


> When a ZipArchive is written, in 7.0 it uses
> (File named: aFileName)
> writeStreamDo: [...]
> In 6.1, it uses
> StandardFileStream
> forceNewFileNamed: aFileName
> do: [...]
>

Well, the equivalent of this is:

f := File named: aFileName.
f exists ifTrue: [ f delete ].
f writeStreamDo: [...].

or

f := File named: aFileName.
f writeStreamDo: [ :stream |
   stream truncate.
   ...
].

This is, I reckon, slightly more verbose.
On the other side I personally think that "forceNewFileNamed:" is not so
useful nor general, maybe for scripting purposes.
I think that libraries (as Zip does here for example) should use that with
care, since silently overriding a file can be considered nasty :).

Besides, taking a Pharo5 image, users of forceNewFileNamed: and
forceNewFileNamed:do: are few, mostly tests.
And several of the users do override a file silently (and dangerously IMHO)
since the do not make it explicit to the user :/.
Take for example in pharo5

MIMEDocument >> saveToFile: anAbsolutePathString
FileStream forceNewFileNamed: anAbsolutePathString do: [ :str |
str binary.
str nextPutAll: (self contents) ].

or Fuel's

FLSqueakPlatform >> fileNamed: aFilename writeStreamDo: aBlock
^ FileStream
forceNewFileNamed: aFilename
do: [ :stream |
stream binary.
aBlock value: stream ]

which override the file but the method name does not indicate it. What if
the file being overwritten had important data?

which is a small but significant difference leading to corrupted zip
> archives when overwriting existing files. File does not have equivalent
> protocol to #forceNewFileNamed... so it is pretty likely that there are
> more
> bugs creeping there. Upon a cursory inspection I see that for example
> FLFileStreamStrategy will suffer from the same problem when writing to
> existing files.
>
> Is there any documentation on the reasoning of depreciating
> StandardFileStream and who is working on this? I'm really new to pharo
> development... Submitting "patches" without knowing the original design
> rationale seems not very productive.
>

I've made part of the effort myself, Sven did a bit part too, and many
other people whose names aren't coming out of the top of my head (sorry!).

Part of the migration guide is here:
https://pharoweekly.wordpress.com/2018/03/19/new-files-in-pharo-migration-guide-how-tos-and-examples/
Paver wrote another one that I cannot find right now.

There are many emails around discussing the rationale behind this. But in a
couple of words the reason was cleaning up.
We had for example the concerns of data encoding and buffering repeated
between sockets and file management.
And also this was using subclasses, which tend to explode with combinations.
We refactored this to use a decorator schema which is far more composable
and modular, and lets us reuse cross cutting concerns between memory
streams, file streams, sockets, etc.

Guille


Re: [Pharo-users] Usage of String and FileLocator to reference files in ZipArchive (and probably other places)

2018-11-25 Thread Hans-Martin
I found a related problem which indicates that the depreciation of
StandardFileStream in favor of File is incomplete and probably premature:

When a ZipArchive is written, in 7.0 it uses
(File named: aFileName)
writeStreamDo: [...]
In 6.1, it uses
StandardFileStream 
forceNewFileNamed: aFileName
do: [...]
which is a small but significant difference leading to corrupted zip
archives when overwriting existing files. File does not have equivalent
protocol to #forceNewFileNamed... so it is pretty likely that there are more
bugs creeping there. Upon a cursory inspection I see that for example
FLFileStreamStrategy will suffer from the same problem when writing to
existing files.

Is there any documentation on the reasoning of depreciating
StandardFileStream and who is working on this? I'm really new to pharo
development... Submitting "patches" without knowing the original design
rationale seems not very productive.

Cheers,
Hans-Martin



--
Sent from: http://forum.world.st/Pharo-Smalltalk-Users-f1310670.html



Re: [Pharo-users] Usage of String and FileLocator to reference files in ZipArchive (and probably other places)

2018-11-17 Thread Sven Van Caekenberghe





ok, I had a quick look.

the whole ZIP archive code has quite close ties with File (and vice versa: a 
lot of methods on File are only used by that code), while I don't think this is 
strictly necessary.

I know that this has historical reasons.

some more work needs to be done here.

> On 17 Nov 2018, at 17:42, Hans-Martin  wrote:
> 
> Ok here's an example:
> 
> ZipArchive isZipArchive: FileLocator temp / 'nonexist.zip'
> 
> In 6.1, this brings up the nonexistant file dialog if the file does not
> exist, otherwise it returns true or false depending on whether the file is a
> zip archive.
> In 7.0, you get an error "Instance of FileLocator did not understand
> #utf8Encoded" independent of whether the file exists, is a zip archive or
> not :-)
> 
> Cheers,
> Hans-Martin
> 
> 
> 
> --
> Sent from: http://forum.world.st/Pharo-Smalltalk-Users-f1310670.html
> 




Re: [Pharo-users] Usage of String and FileLocator to reference files in ZipArchive (and probably other places)

2018-11-17 Thread Hans-Martin
Ok here's an example:

ZipArchive isZipArchive: FileLocator temp / 'nonexist.zip'

In 6.1, this brings up the nonexistant file dialog if the file does not
exist, otherwise it returns true or false depending on whether the file is a
zip archive.
In 7.0, you get an error "Instance of FileLocator did not understand
#utf8Encoded" independent of whether the file exists, is a zip archive or
not :-)

Cheers,
Hans-Martin



--
Sent from: http://forum.world.st/Pharo-Smalltalk-Users-f1310670.html



Re: [Pharo-users] Usage of String and FileLocator to reference files in ZipArchive (and probably other places)

2018-11-17 Thread Sven Van Caekenberghe



> On 17 Nov 2018, at 16:04, Hans-Martin  wrote:
> 
> Hi, I've stumbled upon a 7.0 incompatibility with regards to how file names
> are treated.
> In 6.1, I can use Strings and FileLocators interchangeably to create and
> read Zip archives. That's because StandardFileStream does the magic behind
> the screnes.

Could you give a code example ? What API/methods are you talking about ?

What worked before and does not work anymore ?

> In 7.0, StandardFileStream has been deprecated, and "File
> openForReadFileNamed:" is used instead. That method does not handle
> FileLocators, so code that uses FileLocators to open Zip archives ceased to
> work.

*NO* you should not use File directly.

Just use FileReference instances (however you create them) and work from there. 
If you got a string path, use #asFileReference.

  FileLocator desktop / 'foo.txt readStreamDo: [ :in | in upToEnd ]

MyObject>>#readFromFile: file
  ^ file asFileReference readStreamDo: [ :in | self readFromStream: in ]

> What's the preferred solution? It strikes me as illogical that objects which
> were made to reference files can't be used to reference files in these
> contexts. However, there may be a conscious design decision behind this
> change that's just not obvious to me.
> 
> Cheers,
> Hans-Martin
> 
> 
> 
> --
> Sent from: http://forum.world.st/Pharo-Smalltalk-Users-f1310670.html
> 




[Pharo-users] Usage of String and FileLocator to reference files in ZipArchive (and probably other places)

2018-11-17 Thread Hans-Martin
Hi, I've stumbled upon a 7.0 incompatibility with regards to how file names
are treated.
In 6.1, I can use Strings and FileLocators interchangeably to create and
read Zip archives. That's because StandardFileStream does the magic behind
the screnes.
In 7.0, StandardFileStream has been deprecated, and "File
openForReadFileNamed:" is used instead. That method does not handle
FileLocators, so code that uses FileLocators to open Zip archives ceased to
work.

What's the preferred solution? It strikes me as illogical that objects which
were made to reference files can't be used to reference files in these
contexts. However, there may be a conscious design decision behind this
change that's just not obvious to me.

Cheers,
Hans-Martin



--
Sent from: http://forum.world.st/Pharo-Smalltalk-Users-f1310670.html