The Trunk: Network-tfel.184.mcz

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

The Trunk: Network-tfel.184.mcz

commits-2
Tim Felgentreff uploaded a new version of Network to project The Trunk:
http://source.squeak.org/trunk/Network-tfel.184.mcz

==================== Summary ====================

Name: Network-tfel.184
Author: tfel
Time: 30 August 2016, 11:53:57.083946 am
UUID: b90acf7c-5796-a347-8939-59955c0588dd
Ancestors: Network-tfel.183, Network-ul.183

merge a few fixes from Squeakland Etoys.
- ServerDirectories always use forward slashes, even on windows
- FTPClient connections should go through the NetNameResolver
- sockets can only accept if they are connected.

=============== Diff against Network-ul.183 ===============

Item was changed:
  ----- Method: FTPClient>>openPassiveDataConnection (in category 'private protocol') -----
  openPassiveDataConnection
  | portInfo list dataPort remoteHostAddress |
  self sendCommand: 'PASV'.
  self lookForCode: 227 ifDifferent: [:response | (TelnetProtocolError protocolInstance: self) signal: 'Could not enter passive mode: ' , response].
-
  portInfo := (self lastResponse findTokens: '()') at: 2.
  list := portInfo findTokens: ','.
+ remoteHostAddress := NetNameResolver addressForName: (list at: 1)
+ , '.'
+ , (list at: 2) , '.'
+ , (list at: 3) , '.'
+ , (list at: 4) timeout: 30.
- remoteHostAddress := ByteArray
- with: (list at: 1) asNumber
- with: (list at: 2) asNumber
- with: (list at: 3) asNumber
- with: (list at: 4) asNumber.
  dataPort := (list at: 5) asNumber * 256 + (list at: 6) asNumber.
+ self openDataSocket: remoteHostAddress port: dataPort!
- self openDataSocket: remoteHostAddress port: dataPort
- !

Item was changed:
  ----- Method: ServerDirectory>>isRoot (in category 'testing') -----
  isRoot
+ ^ directory = '/'!
- ^directory = (String with: self pathNameDelimiter)!

Item was changed:
  ----- Method: Socket>>waitForAcceptFor:ifTimedOut: (in category 'waiting') -----
  waitForAcceptFor: timeout ifTimedOut: timeoutBlock
  "Wait and accept an incoming connection"
  self waitForConnectionFor: timeout ifTimedOut: [^timeoutBlock value].
+ ^self isConnected
+ ifTrue:[self accept]
+ !
- ^self accept!


Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Network-tfel.184.mcz

Levente Uzonyi
On Tue, 30 Aug 2016, [hidden email] wrote:

> Tim Felgentreff uploaded a new version of Network to project The Trunk:
> http://source.squeak.org/trunk/Network-tfel.184.mcz
>
> ==================== Summary ====================
>
> Name: Network-tfel.184
> Author: tfel
> Time: 30 August 2016, 11:53:57.083946 am
> UUID: b90acf7c-5796-a347-8939-59955c0588dd
> Ancestors: Network-tfel.183, Network-ul.183
>
> merge a few fixes from Squeakland Etoys.
> - ServerDirectories always use forward slashes, even on windows
> - FTPClient connections should go through the NetNameResolver
> - sockets can only accept if they are connected.
>
> =============== Diff against Network-ul.183 ===============
>
> Item was changed:
>  ----- Method: FTPClient>>openPassiveDataConnection (in category 'private protocol') -----
>  openPassiveDataConnection
>   | portInfo list dataPort remoteHostAddress |
>   self sendCommand: 'PASV'.
>   self lookForCode: 227 ifDifferent: [:response | (TelnetProtocolError protocolInstance: self) signal: 'Could not enter passive mode: ' , response].
> -
>   portInfo := (self lastResponse findTokens: '()') at: 2.
>   list := portInfo findTokens: ','.
> + remoteHostAddress := NetNameResolver addressForName: (list at: 1)
> + , '.'
> + , (list at: 2) , '.'
> + , (list at: 3) , '.'
> + , (list at: 4) timeout: 30.
> - remoteHostAddress := ByteArray
> - with: (list at: 1) asNumber
> - with: (list at: 2) asNumber
> - with: (list at: 3) asNumber
> - with: (list at: 4) asNumber.
>   dataPort := (list at: 5) asNumber * 256 + (list at: 6) asNumber.
> + self openDataSocket: remoteHostAddress port: dataPort!
> - self openDataSocket: remoteHostAddress port: dataPort
> - !
>
> Item was changed:
>  ----- Method: ServerDirectory>>isRoot (in category 'testing') -----
>  isRoot
> + ^ directory = '/'!
> - ^directory = (String with: self pathNameDelimiter)!
>
> Item was changed:
>  ----- Method: Socket>>waitForAcceptFor:ifTimedOut: (in category 'waiting') -----
>  waitForAcceptFor: timeout ifTimedOut: timeoutBlock
>   "Wait and accept an incoming connection"
>   self waitForConnectionFor: timeout ifTimedOut: [^timeoutBlock value].
> + ^self isConnected

Does this change actually do anything? Won't the previous line return when
the socket is not connected?

Levente

> + ifTrue:[self accept]
> + !
> - ^self accept!
>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Network-tfel.184.mcz

timfelgentreff
You're right, I overlooked this.

I just realized how unfortunate the diff format is in this case. None of these methods were changed by me, I merely merged from Squeakland and tried to figure out what to keep :)


On Tue, 30 Aug 2016 at 23:01 Levente Uzonyi <[hidden email]> wrote:
On Tue, 30 Aug 2016, [hidden email] wrote:

> Tim Felgentreff uploaded a new version of Network to project The Trunk:
> http://source.squeak.org/trunk/Network-tfel.184.mcz
>
> ==================== Summary ====================
>
> Name: Network-tfel.184
> Author: tfel
> Time: 30 August 2016, 11:53:57.083946 am
> UUID: b90acf7c-5796-a347-8939-59955c0588dd
> Ancestors: Network-tfel.183, Network-ul.183
>
> merge a few fixes from Squeakland Etoys.
> - ServerDirectories always use forward slashes, even on windows
> - FTPClient connections should go through the NetNameResolver
> - sockets can only accept if they are connected.
>
> =============== Diff against Network-ul.183 ===============
>
> Item was changed:
>  ----- Method: FTPClient>>openPassiveDataConnection (in category 'private protocol') -----
>  openPassiveDataConnection
>       | portInfo list dataPort remoteHostAddress |
>       self sendCommand: 'PASV'.
>       self lookForCode: 227 ifDifferent: [:response | (TelnetProtocolError protocolInstance: self) signal: 'Could not enter passive mode: ' , response].
> -
>       portInfo := (self lastResponse findTokens: '()') at: 2.
>       list := portInfo findTokens: ','.
> +     remoteHostAddress := NetNameResolver addressForName: (list at: 1)
> +                                     , '.'
> +                                     , (list at: 2) , '.'
> +                                     , (list at: 3) , '.'
> +                                     , (list at: 4) timeout: 30.
> -     remoteHostAddress := ByteArray
> -             with: (list at: 1) asNumber
> -             with: (list at: 2) asNumber
> -             with: (list at: 3) asNumber
> -             with: (list at: 4) asNumber.
>       dataPort := (list at: 5) asNumber * 256 + (list at: 6) asNumber.
> +     self openDataSocket: remoteHostAddress port: dataPort!
> -     self openDataSocket: remoteHostAddress port: dataPort
> - !
>
> Item was changed:
>  ----- Method: ServerDirectory>>isRoot (in category 'testing') -----
>  isRoot
> +     ^ directory = '/'!
> -     ^directory = (String with: self pathNameDelimiter)!
>
> Item was changed:
>  ----- Method: Socket>>waitForAcceptFor:ifTimedOut: (in category 'waiting') -----
>  waitForAcceptFor: timeout ifTimedOut: timeoutBlock
>       "Wait and accept an incoming connection"
>       self waitForConnectionFor: timeout ifTimedOut: [^timeoutBlock value].
> +     ^self isConnected

Does this change actually do anything? Won't the previous line return when
the socket is not connected?

Levente

> +             ifTrue:[self accept]
> + !
> -     ^self accept!
>
>
>



Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Network-tfel.184.mcz

Levente Uzonyi
Do you happen to use pretty diffs? If so, you should turn that off. There
were quite a few changes which did nothing but reverted := to _. We should
revert those again.

Levente

On Wed, 31 Aug 2016, Tim Felgentreff wrote:

> You're right, I overlooked this.
> I just realized how unfortunate the diff format is in this case. None of these methods were changed by me, I merely merged from Squeakland and
> tried to figure out what to keep :)
>
>
> On Tue, 30 Aug 2016 at 23:01 Levente Uzonyi <[hidden email]> wrote:
>       On Tue, 30 Aug 2016, [hidden email] wrote:
>
>       > Tim Felgentreff uploaded a new version of Network to project The Trunk:
>       > http://source.squeak.org/trunk/Network-tfel.184.mcz
>       >
>       > ==================== Summary ====================
>       >
>       > Name: Network-tfel.184
>       > Author: tfel
>       > Time: 30 August 2016, 11:53:57.083946 am
>       > UUID: b90acf7c-5796-a347-8939-59955c0588dd
>       > Ancestors: Network-tfel.183, Network-ul.183
>       >
>       > merge a few fixes from Squeakland Etoys.
>       > - ServerDirectories always use forward slashes, even on windows
>       > - FTPClient connections should go through the NetNameResolver
>       > - sockets can only accept if they are connected.
>       >
>       > =============== Diff against Network-ul.183 ===============
>       >
>       > Item was changed:
>       >  ----- Method: FTPClient>>openPassiveDataConnection (in category 'private protocol') -----
>       >  openPassiveDataConnection
>       >       | portInfo list dataPort remoteHostAddress |
>       >       self sendCommand: 'PASV'.
>       >       self lookForCode: 227 ifDifferent: [:response | (TelnetProtocolError protocolInstance: self) signal: 'Could not enter passive
>       mode: ' , response].
>       > -
>       >       portInfo := (self lastResponse findTokens: '()') at: 2.
>       >       list := portInfo findTokens: ','.
>       > +     remoteHostAddress := NetNameResolver addressForName: (list at: 1)
>       > +                                     , '.'
>       > +                                     , (list at: 2) , '.'
>       > +                                     , (list at: 3) , '.'
>       > +                                     , (list at: 4) timeout: 30.
>       > -     remoteHostAddress := ByteArray
>       > -             with: (list at: 1) asNumber
>       > -             with: (list at: 2) asNumber
>       > -             with: (list at: 3) asNumber
>       > -             with: (list at: 4) asNumber.
>       >       dataPort := (list at: 5) asNumber * 256 + (list at: 6) asNumber.
>       > +     self openDataSocket: remoteHostAddress port: dataPort!
>       > -     self openDataSocket: remoteHostAddress port: dataPort
>       > - !
>       >
>       > Item was changed:
>       >  ----- Method: ServerDirectory>>isRoot (in category 'testing') -----
>       >  isRoot
>       > +     ^ directory = '/'!
>       > -     ^directory = (String with: self pathNameDelimiter)!
>       >
>       > Item was changed:
>       >  ----- Method: Socket>>waitForAcceptFor:ifTimedOut: (in category 'waiting') -----
>       >  waitForAcceptFor: timeout ifTimedOut: timeoutBlock
>       >       "Wait and accept an incoming connection"
>       >       self waitForConnectionFor: timeout ifTimedOut: [^timeoutBlock value].
>       > +     ^self isConnected
>
>       Does this change actually do anything? Won't the previous line return when
>       the socket is not connected?
>
>       Levente
>
>       > +             ifTrue:[self accept]
>       > + !
>       > -     ^self accept!
>       >
>       >
>       >
>
>
>

Reply | Threaded
Open this post in threaded view
|

Trunk Ancestry (was Re: [squeak-dev] The Trunk: Network-tfel.184.mcz)

Tobias Pape

On 31.08.2016, at 22:01, Levente Uzonyi <[hidden email]> wrote:

> Do you happen to use pretty diffs? If so, you should turn that off. There were quite a few changes which did nothing but reverted := to _. We should revert those again.

These are not in the current versions.
The diffs are bogus. Take for example the recently updated Sound package.
Part of the last diff we saw was

==================== Summary ====================

Name: Sound-tfel.58
Author: tfel
Time: 21 August 2016, 4:48:44.407391 pm
UUID: 50e3fdac-34a3-412c-8cd6-bc672a2424ae
Ancestors: Sound-mt.57, Sound-tfel.57

merge trunk

=============== Diff against Sound-mt.57 ===============

Item was changed:
 ----- Method: AbstractSound class>>fileInSoundLibraryNamed: (in category 'sound library-file in/out') -----
 fileInSoundLibraryNamed: fileName
  "File in the sound library with the given file name, and add its contents to the current sound library."

  | s newSounds |
+ s _ FileStream readOnlyFileNamed: fileName.
+ newSounds _ s fileInObjectAndCode.
- s := FileStream oldFileNamed: fileName.
- newSounds := s fileInObjectAndCode.
  s close.
  newSounds associationsDo:
  [:assoc | self storeFiledInSound: assoc value named: assoc key].
  AbstractSound updateScorePlayers.
  Smalltalk garbageCollect.  "Large objects may have been released"
 !


Yet, in the _latest_ Package Sound-tfel.59, the method looks like that:

fileInSoundLibraryNamed: fileName
        "File in the sound library with the given file name, and add its contents to the current sound library."

        | s newSounds |
        s := FileStream readOnlyFileNamed: fileName.
        newSounds := s fileInObjectAndCode.

(besides, with a comment stamp of: 'kks 9/27/2007 16:25 sound library-file in/out'  )


=======================================================================


So, what tim did was to commit the most recent version of the changes he had and retroactively pulling in the
ancestors. The only problem here is SqueakSource, which does not shut up about this. Those Diffs are just
meaningless.

Best regards
        -Tobias






>
> Levente
>
> On Wed, 31 Aug 2016, Tim Felgentreff wrote:
>
>> You're right, I overlooked this.
>> I just realized how unfortunate the diff format is in this case. None of these methods were changed by me, I merely merged from Squeakland and
>> tried to figure out what to keep :)
>> On Tue, 30 Aug 2016 at 23:01 Levente Uzonyi <[hidden email]> wrote:
>>      On Tue, 30 Aug 2016, [hidden email] wrote:
>>
>>      > Tim Felgentreff uploaded a new version of Network to project The Trunk:
>>      > http://source.squeak.org/trunk/Network-tfel.184.mcz
>>      >
>>      > ==================== Summary ====================
>>      >
>>      > Name: Network-tfel.184
>>      > Author: tfel
>>      > Time: 30 August 2016, 11:53:57.083946 am
>>      > UUID: b90acf7c-5796-a347-8939-59955c0588dd
>>      > Ancestors: Network-tfel.183, Network-ul.183
>>      >
>>      > merge a few fixes from Squeakland Etoys.
>>      > - ServerDirectories always use forward slashes, even on windows
>>      > - FTPClient connections should go through the NetNameResolver
>>      > - sockets can only accept if they are connected.
>>      >
>>      > =============== Diff against Network-ul.183 ===============
>>      >
>>      > Item was changed:
>>      >  ----- Method: FTPClient>>openPassiveDataConnection (in category 'private protocol') -----
>>      >  openPassiveDataConnection
>>      >       | portInfo list dataPort remoteHostAddress |
>>      >       self sendCommand: 'PASV'.
>>      >       self lookForCode: 227 ifDifferent: [:response | (TelnetProtocolError protocolInstance: self) signal: 'Could not enter passive
>>      mode: ' , response].
>>      > -
>>      >       portInfo := (self lastResponse findTokens: '()') at: 2.
>>      >       list := portInfo findTokens: ','.
>>      > +     remoteHostAddress := NetNameResolver addressForName: (list at: 1)
>>      > +                                     , '.'
>>      > +                                     , (list at: 2) , '.'
>>      > +                                     , (list at: 3) , '.'
>>      > +                                     , (list at: 4) timeout: 30.
>>      > -     remoteHostAddress := ByteArray
>>      > -             with: (list at: 1) asNumber
>>      > -             with: (list at: 2) asNumber
>>      > -             with: (list at: 3) asNumber
>>      > -             with: (list at: 4) asNumber.
>>      >       dataPort := (list at: 5) asNumber * 256 + (list at: 6) asNumber.
>>      > +     self openDataSocket: remoteHostAddress port: dataPort!
>>      > -     self openDataSocket: remoteHostAddress port: dataPort
>>      > - !
>>      >
>>      > Item was changed:
>>      >  ----- Method: ServerDirectory>>isRoot (in category 'testing') -----
>>      >  isRoot
>>      > +     ^ directory = '/'!
>>      > -     ^directory = (String with: self pathNameDelimiter)!
>>      >
>>      > Item was changed:
>>      >  ----- Method: Socket>>waitForAcceptFor:ifTimedOut: (in category 'waiting') -----
>>      >  waitForAcceptFor: timeout ifTimedOut: timeoutBlock
>>      >       "Wait and accept an incoming connection"
>>      >       self waitForConnectionFor: timeout ifTimedOut: [^timeoutBlock value].
>>      > +     ^self isConnected
>>
>>      Does this change actually do anything? Won't the previous line return when
>>      the socket is not connected?
>>
>>      Levente
>>
>>      > +             ifTrue:[self accept]
>>      > + !
>>      > -     ^self accept!
>>      >
>>      >
>>      >



Reply | Threaded
Open this post in threaded view
|

Re: Trunk Ancestry (was Re: [squeak-dev] The Trunk: Network-tfel.184.mcz)

Levente Uzonyi
That's correct. The only method with underscore assignment, other than
those which have it commented out, is DateAndTime >> #<.

Levente

On Wed, 31 Aug 2016, Tobias Pape wrote:

>
> On 31.08.2016, at 22:01, Levente Uzonyi <[hidden email]> wrote:
>
>> Do you happen to use pretty diffs? If so, you should turn that off. There were quite a few changes which did nothing but reverted := to _. We should revert those again.
>
> These are not in the current versions.
> The diffs are bogus. Take for example the recently updated Sound package.
> Part of the last diff we saw was
>
> ==================== Summary ====================
>
> Name: Sound-tfel.58
> Author: tfel
> Time: 21 August 2016, 4:48:44.407391 pm
> UUID: 50e3fdac-34a3-412c-8cd6-bc672a2424ae
> Ancestors: Sound-mt.57, Sound-tfel.57
>
> merge trunk
>
> =============== Diff against Sound-mt.57 ===============
>
> Item was changed:
> ----- Method: AbstractSound class>>fileInSoundLibraryNamed: (in category 'sound library-file in/out') -----
> fileInSoundLibraryNamed: fileName
> "File in the sound library with the given file name, and add its contents to the current sound library."
>
> | s newSounds |
> + s _ FileStream readOnlyFileNamed: fileName.
> + newSounds _ s fileInObjectAndCode.
> - s := FileStream oldFileNamed: fileName.
> - newSounds := s fileInObjectAndCode.
> s close.
> newSounds associationsDo:
> [:assoc | self storeFiledInSound: assoc value named: assoc key].
> AbstractSound updateScorePlayers.
> Smalltalk garbageCollect.  "Large objects may have been released"
> !
>
>
> Yet, in the _latest_ Package Sound-tfel.59, the method looks like that:
>
> fileInSoundLibraryNamed: fileName
> "File in the sound library with the given file name, and add its contents to the current sound library."
>
> | s newSounds |
> s := FileStream readOnlyFileNamed: fileName.
> newSounds := s fileInObjectAndCode.
>
> (besides, with a comment stamp of: 'kks 9/27/2007 16:25 sound library-file in/out'  )
>
>
> =======================================================================
>
>
> So, what tim did was to commit the most recent version of the changes he had and retroactively pulling in the
> ancestors. The only problem here is SqueakSource, which does not shut up about this. Those Diffs are just
> meaningless.
>
> Best regards
> -Tobias
>
>
>
>
>
>
>>
>> Levente
>>
>> On Wed, 31 Aug 2016, Tim Felgentreff wrote:
>>
>>> You're right, I overlooked this.
>>> I just realized how unfortunate the diff format is in this case. None of these methods were changed by me, I merely merged from Squeakland and
>>> tried to figure out what to keep :)
>>> On Tue, 30 Aug 2016 at 23:01 Levente Uzonyi <[hidden email]> wrote:
>>>      On Tue, 30 Aug 2016, [hidden email] wrote:
>>>
>>>     > Tim Felgentreff uploaded a new version of Network to project The Trunk:
>>>     > http://source.squeak.org/trunk/Network-tfel.184.mcz
>>>     >
>>>     > ==================== Summary ====================
>>>     >
>>>     > Name: Network-tfel.184
>>>     > Author: tfel
>>>     > Time: 30 August 2016, 11:53:57.083946 am
>>>     > UUID: b90acf7c-5796-a347-8939-59955c0588dd
>>>     > Ancestors: Network-tfel.183, Network-ul.183
>>>     >
>>>     > merge a few fixes from Squeakland Etoys.
>>>     > - ServerDirectories always use forward slashes, even on windows
>>>     > - FTPClient connections should go through the NetNameResolver
>>>     > - sockets can only accept if they are connected.
>>>     >
>>>     > =============== Diff against Network-ul.183 ===============
>>>     >
>>>     > Item was changed:
>>>     >  ----- Method: FTPClient>>openPassiveDataConnection (in category 'private protocol') -----
>>>     >  openPassiveDataConnection
>>>     >       | portInfo list dataPort remoteHostAddress |
>>>     >       self sendCommand: 'PASV'.
>>>     >       self lookForCode: 227 ifDifferent: [:response | (TelnetProtocolError protocolInstance: self) signal: 'Could not enter passive
>>>      mode: ' , response].
>>>     > -
>>>     >       portInfo := (self lastResponse findTokens: '()') at: 2.
>>>     >       list := portInfo findTokens: ','.
>>>     > +     remoteHostAddress := NetNameResolver addressForName: (list at: 1)
>>>     > +                                     , '.'
>>>     > +                                     , (list at: 2) , '.'
>>>     > +                                     , (list at: 3) , '.'
>>>     > +                                     , (list at: 4) timeout: 30.
>>>     > -     remoteHostAddress := ByteArray
>>>     > -             with: (list at: 1) asNumber
>>>     > -             with: (list at: 2) asNumber
>>>     > -             with: (list at: 3) asNumber
>>>     > -             with: (list at: 4) asNumber.
>>>     >       dataPort := (list at: 5) asNumber * 256 + (list at: 6) asNumber.
>>>     > +     self openDataSocket: remoteHostAddress port: dataPort!
>>>     > -     self openDataSocket: remoteHostAddress port: dataPort
>>>     > - !
>>>     >
>>>     > Item was changed:
>>>     >  ----- Method: ServerDirectory>>isRoot (in category 'testing') -----
>>>     >  isRoot
>>>     > +     ^ directory = '/'!
>>>     > -     ^directory = (String with: self pathNameDelimiter)!
>>>     >
>>>     > Item was changed:
>>>     >  ----- Method: Socket>>waitForAcceptFor:ifTimedOut: (in category 'waiting') -----
>>>     >  waitForAcceptFor: timeout ifTimedOut: timeoutBlock
>>>     >       "Wait and accept an incoming connection"
>>>     >       self waitForConnectionFor: timeout ifTimedOut: [^timeoutBlock value].
>>>     > +     ^self isConnected
>>>
>>>      Does this change actually do anything? Won't the previous line return when
>>>      the socket is not connected?
>>>
>>>      Levente
>>>
>>>     > +             ifTrue:[self accept]
>>>     > + !
>>>     > -     ^self accept!
>>>     >
>>>     >
>>>     >
>
>
>
>