[squeak-dev] How to rewrite a license restricted method?

This is going to be a rather long email but I ask you to at least skim
if you have even a little interest in the relicensing/Squeak 4.0 effort.

Thanks to the efforts of many including Yoshiki Ohshima and Matthew
Fulmer we have made some real progress on a 'license clean' version of
Squeak.  But we are now at the phase where we have some code for which
it appears we are not going to get a relicensing agreement.  So at this
point it's either remove it or replace it.

Thanks to Matthew publishing the history tools I've started to try to
help out with this and have quickly learned that I really don't know
what to do.  I'm going to present an example here and I'm asking you for
your considered opinion.

SequenceableCollection>>asStringWithCr

This method goes back to Squeak1.0 and has 5 versions leading up to what
is today in Squeak3.10.2-7197/Squeak4.0alpha.  It is the second version
by Douglas McPherson that has the problem (we have not received a
relicensing agreement from Douglas).  I'm going to paste in, oldest to
newest, all versions so we can discuss this concretely.  In each I will
number the version in front of the containing filename for discussion
convenience and mark changed lines (relative to the earlier version)
with a + in the first column.  The filenames used are those as
distributed with the Matthew/Yoshiki history tools; sometimes these
don't directly map to the original updates (think updates that load
MCZs).

1) SqueakV1.sources:

asStringWithCr
        "Convert to a string with returns between items.  Elements are usually
strings.
         Useful for labels for PopUpMenus."
        | labelStream |
        labelStream _ WriteStream on: (String new: 200).
        self do: [:each |
                (each isKindOf: String)
                        ifTrue: [labelStream nextPutAll: each; cr]
                        ifFalse: [each printOn: labelStream; cr]].
        self size > 0 ifTrue: [labelStream skip: -1].
        ^ labelStream contents!

2) 442TweaksForBeta-di.cs:

!SequenceableCollection methodsFor: 'converting' stamp: 'djm 11/20/1998
05:44'!
asStringWithCr
        "Convert to a string with returns between items.  Elements are
usually strings.
         Useful for labels for PopUpMenus."
        | labelStream |
        labelStream _ WriteStream on: (String new: 200).
        self do: [:each |
                (each isKindOf: String)
                        ifTrue: [labelStream nextPutAll: each; cr]
+ ifFalse: [each printOn: labelStream. labelStream cr]].
        self size > 0 ifTrue: [labelStream skip: -1].
        ^ labelStream contents! !

3) 5994-004-systemMod.cs:

!SequenceableCollection methodsFor: 'converting' stamp: 'yo 8/28/2002
15:39'!
asStringWithCr
        "Convert to a string with returns between items.  Elements are
usually strings.
         Useful for labels for PopUpMenus."
        | labelStream |
        labelStream _ WriteStream on: (String new: 200).
        self do: [:each |
+ (each isKindOf: AbstractString)
                        ifTrue: [labelStream nextPutAll: each; cr]
                        ifFalse: [each printOn: labelStream. labelStream cr]].
        self size > 0 ifTrue: [labelStream skip: -1].
        ^ labelStream contents! !

4) 6651ExternalCleanup.cs:

!SequenceableCollection methodsFor: 'converting' stamp: 'ar 4/10/2005
18:02'!
asStringWithCr
        "Convert to a string with returns between items.  Elements are
usually strings.
         Useful for labels for PopUpMenus."
        | labelStream |
        labelStream _ WriteStream on: (String new: 200).
        self do: [:each |
+ each isString
                        ifTrue: [labelStream nextPutAll: each; cr]
                        ifFalse: [each printOn: labelStream. labelStream cr]].
        self size > 0 ifTrue: [labelStream skip: -1].
        ^ labelStream contents! !

5) Squeak39g-7056+3102-7179.changes:

!SequenceableCollection methodsFor: 'converting' stamp: 'ar 4/10/2005
18:02' prior: 29748463!
asStringWithCr
        "Convert to a string with returns between items.  Elements are
usually strings.
         Useful for labels for PopUpMenus."
        | labelStream |
+ labelStream := WriteStream on: (String new: 200).
        self do: [:each |
                each isString
                        ifTrue: [labelStream nextPutAll: each; cr]
                        ifFalse: [each printOn: labelStream. labelStream cr]].
        self size > 0 ifTrue: [labelStream skip: -1].
        ^ labelStream contents! !

Now as I said above the problem with with version 2.  As you can see
each of these changes is at most one line and by and large frankly
rather obvious.  The change in 2 is clearly a bug fix and the
straightforward solution.

So the question is what do I do with this method, which is currently
distributed as version 5, so that it does not infringe on djm's rights
to maintain version 2 as SqueakL?  Admittedly it's unlikely that djm
actually wants this right, but lacking a clear statement to the
contrary, it's the default we have to assume.

How much of a change is a significant change?

Should I delete the current implementation entirely and rewrite it from
scratch?  If so, how much am I allowed to know about the prior
implementations?

Ken

Hi Ken,

Ken Causey wrote:
>
> Should I delete the current implementation entirely and rewrite it from
> scratch?  If so, how much am I allowed to know about the prior
> implementations?
>

I would naively suggest that you or someone with knowledge of the method
writes a set of unit tests and a comment, then someone who hasn't seen
the method could implement it. Of course finding and ensuring someone
hasn't seen the method may be impossible. And, many times the
implementation may end up the same anyway.

- Zulq

On 08.02.2009, at 20:26, Ken Causey wrote:

If I pointed out the flaws that need to be fixed in this version -  
namely, replace "_" by ":=", use #isStrign instead of #isKindOf:, and  
make sure #cr is sent to the labelStream, I am certain we would end up  
exactly with this:


So IMHO we do not need to worry about this particular method at all  
since the non-relicensed contribution is a trivial bug fix.

- Bert -

2009/2/9 Bert Freudenberg <[hidden email]>:
How about this one:

 asStringWithCr
        "Convert to a string with returns between items.  Elements are
usually strings.
         Useful for labels for PopUpMenus."
        ^ String streamContents: [:str |
            self do: [:each | str nextPutAll: each asString ]
separatedBy: [ str cr ]
        ]

Btw, i was surprised to see in String>>printOn: something else than
'stream nextPutAll: self' , otherwise we could simply put
'each printOn: str' in the method above.

--
Best regards,
Igor Stasenko AKA sig.

  Hi, Ken,

  At least, For the methods that also exists in Etoys 4.0, I don't
think it is fruitful to bring in more scrutiny...

  In regards to asStringWithCr, it would certainly classified as
trivial.  (Does anybody use the method with a collection whose
elements are not strings, BTW?)

> Should I delete the current implementation entirely and rewrite it from
> scratch?

  I don't think so.

-- Yoshiki

On Mon, Feb 09, 2009 at 10:56:36AM -0800, Yoshiki Ohshima wrote:
That's a good point. If the audit lists a method that is part of
etoys 4.0, a reasonable fix is to file it out of etoys 4.0 and
into 3.10.

http://tinlizzie.org/olpc/etoys-dev-4.0.zip

--
Matthew Fulmer -- http://mtfulmer.wordpress.com/

On Mon, Feb 09, 2009 at 02:35:52PM -0500, Matthew Fulmer wrote:
If you take this approach and find that the source code is the
same in etoys as in 3.10, please include the fix in your
relicense change set anyway. This will make it easier when we
port the relicense to Pharo and Cobalt, since anything included
in a relicensing change set is guaranteed clean.

--
Matthew Fulmer -- http://mtfulmer.wordpress.com/

On Mon, 2009-02-09 at 15:45 +0200, Igor Stasenko wrote: Igor,

I'm going to pick on you so as to head off any possible diversion down
this path, please don't take it personally.

You've missed the point.  I care nothing about this particular method
and don't need assistance rewriting it.  If you are interested in
rewriting methods please join us in this final phase of the licensing
audit.  I'm simply using this as a concrete example to indicate the
issue.

My point is that we need a clear official policy on both the criteria by
which we can simply accept the current version of a method despite one
or more missing licensing agreements relevant to it and the procedure we
should follow when a rewrite is deemed necessary.

The goal of this exercise is not to please you and me.  This is all
about doing due diligence to satisfy everyone else that Squeak 4.0 is
truly a license clean release that anyone can feel safe using and for
which there is no significant legal risk.

Ken

2009/2/9 Ken Causey <[hidden email]>: i see, sorry for hurry with posting 2 cents :)

I remember we had such discussions before , a most 'secure' way was to
have 2 roles - one is implementor, another is overseer. An overseer
looks at problematic method and describing what method should do to
'implementor'. Then implementor makes own implementation without
looking at current implementation at all.
Despite how good it sounds, IMO, it is not really works in practice.
First, since given method already exists in image and its source
available for reading at any time, how any implementor can prove that
he never saw original implementation and claim that new implementation
is based on his own mental effort?
Second, describing a method could also be seen as an illegal act,
because your description is based on knowledge of the method sources.
And knowledge is intellectual property :)

As to me, this is a dead end. You may rewrite the method yourself, you
may say someone else to rewrite method - its merely same thing,
because squeak sources is open to anyone.

So, how hard we try, there will be always a way to blame us in
violating copyrights, because they are flawed.
>From this point, i think its equally safe (equally unsafe) to rewrite
the method or leave it be (if it was already overridden by author who
signed MIT agreement). And, of course, i think there is no difference
whether you looked at 'tainted' method or not - because if they want
to sue you, they will, and any precautions simply will not help.

> Ken
>


--
Best regards,
Igor Stasenko AKA sig.

> As to me, this is a dead end. You may rewrite the method yourself, you
> may say someone else to rewrite method - its merely same thing,
> because squeak sources is open to anyone.
>
> So, how hard we try, there will be always a way to blame us in
> violating copyrights, because they are flawed.

Sure, but that is an argument for doing nothing, and we've already
decided to do the Squeak 4.0 rewrite effort.

So, getting back to the original question, here is a proposal for a
clear official policy on both the criteria by which we can simply
accept the current version of a method despite one or more missing
licensing agreements relevant to it and the procedure we should follow
when a rewrite is deemed necessary.

1. Don't accept any code missing license agreements no matter how trivial.
2. Write a comment describing the purpose of the method. Don't include
implementation details.
3. Delete the source.
4. Remove any prior versions by contributors that haven't signed the agreement.
5. If there is a prior version by someone who has signed the
agreement, bring it forward.
6. Test (not necessarily a formal unit test, but it would be nice).
7. If the test passes, release.
8. If the test failed, post a request to fix the bug. Only accept
submissions from people who are willing to certify that they haven't
looked at a prior implementation. Repeat from step 6.

Then your license defense might center around:
a. how well you've followed the above process
b. whether the submitters claim to cleanliness is authentic

Let me state my concerns another way as it seems I'm not being clear.

My concern is for the unknown person or entity some years from now who
is considering adopting Squeak and sees something about the license
history and has to have some basis for justifying that they have no
reasonable fear of legal repercussions.

It seems to me that whatever we decide we have to clearly fully document
this for years to come.

To make this more concrete it seems to me that the very first customer
we have to please is the Software Freedom Law Center.  Do they not have
some sort of formal policy on this?  If not, shouldn't they want to work
with us to develop one?  This seems like a fundamental starting point at
the very least.

Ken

  Let us not make unnecessary fuss and just carefully see if there is
such problematic methods for Squeak 4.0.

  FWIW, we have not found such methods (that are not removeable or
revertable or just with small fixes) for Etoys 4.0.

  And this is from Matthew's another email but:

> I've done the first step and done a full audit, using Yoshiki's
> tools [1], of all the code
> in Squeak 3.10.2, with the exception of four packages:
> - Monticello
> - SUnit, TestRunner, and SUnitImproved
> - Universes
> - Traits

  BTW, as for SUnit, the (real) original SUnit is flagged as "Public
Domain" and the Squeak version was one time under SqL.  But Sames and
JPerline sent us the signature so we can consider it clean.  And I
didn't think there was anybody for Monticello and Universes and Traits
that matters (I could be wrong).  Even if there are a few, I'd imagine
that getting signature from them is much easier to get ones from much
earlier contributors.

-- Yoshiki

2009/2/10 David Mitchell <[hidden email]>:
the only people who haven't looked at any prior implementation 100%
guaranteed, is those who never used squeak in their life. But once
they run squeak image, there is 99.9% chance that you eventually could
see a source of a random 'license unclean' method. Also, imagine a
developer who knows nothing about squeak to write new implementation..

That's why i think we should not be focused on process of rewrite, we
should simply need to rewrite the unclean methods, regardless of what
you seen or where you been. Because this is completely irrelevant and
can't be proven in any way. Otherwise, as to me there precautions
looks ridiculous to an outsider, as a phrase: i never seen your deep
blue eyes.

> Then your license defense might center around:
> a. how well you've followed the above process
> b. whether the submitters claim to cleanliness is authentic
>
>


--
Best regards,
Igor Stasenko AKA sig.

At Mon, 9 Feb 2009 14:43:32 -0500,
Matthew Fulmer wrote:
>
> If you take this approach and find that the source code is the
> same in etoys as in 3.10, please include the fix in your
> relicense change set anyway.

  I didn't quite understand what you are asking for (is it to me?)

> This will make it easier when we
> port the relicense to Pharo and Cobalt, since anything included
> in a relicensing change set is guaranteed clean.

  We can generate the list of all methods and latest author initials
from Etoys 4.0, and compare it against another system, yes.

-- Yoshiki

>   BTW, as for SUnit, the (real) original SUnit is flagged as "Public
> Domain" and the Squeak version was one time under SqL.  But Sames and
> JPerline sent us the signature so we can consider it clean.  And I
> didn't think there was anybody for Monticello and Universes and Traits
> that matters (I could be wrong).  
1) Monticello is covered (1.5 is already), and whatever MC is in 4.0
will be covered.

2) Traits is fine according to Stephane Ducasse

Audit of Universes, MC1.5 reports that the set of authors, scanning
every method in every version is:

('tonyg','pmm',damiencasssou,'ls','ms,'dc',kph','lr')

searching the list of initials I get:

'ms'->#()
'tonyg'->#(#dataReturnedSignatories)
'kph'->#(#dataNewContributors)
'pmm'->#(#dataMissingSignatories)
'lr'->#(#dataReturnedSignatories)
'ls'->#(#dataReturnedSignatories)
'damiencassou'->#(#dataReturnedSignatories)
'dc'->#(#dataReturnedSignatories))

"ms" ken thinks is Matthew Suen, and Philippe?

Keith

Gulik.


--
http://people.squeakfoundation.org/person/mikevdg
http://gulik.pbwiki.com/

That sounds like a very good approach to this. Wheras I find having to  
get licensing on a trivial change to be anal, I think getting approval  
of an 'anal' group is probably a good plan - unfortunately, as of may  
be the best course for the future of Squeak.

David



On 9-Feb-09, at 14:53, Ken Causey <[hidden email]> wrote:

In this particular case, I'd simply delete the ifFalse:.  The other changes are independent of the change by djm; they could have been made even if version 2) never existed.  The argument for this can be strengthened because in this case it is clear why every change has been made:
    3) to accomodate addition of AbstractString to the class hierarchy  
    4) because #isString looks better and/or is faster
    5) because underscores are no longer allowed for assignment)
... and none of these motivations is in any way related to the change by djm.

Then, I'd advertise for volunteers to re-write the method so that it works when the receiver contains arbitrary objects, not only when it contains Strings (SequenceableCollections?).  A TestCase would help ensure that they get it right.

If this isn't defensible, then I don't know where we would draw the line. 

Of course, there are probably many examples that are more "grey" than this one.

Anyway, thanks for highlighting these issues to the community.  Cheers!
Josh



Ken Causey wrote:

This is going to be a rather long email but I ask you to at least skim
if you have even a little interest in the relicensing/Squeak 4.0 effort.

Thanks to the efforts of many including Yoshiki Ohshima and Matthew
Fulmer we have made some real progress on a 'license clean' version of
Squeak.  But we are now at the phase where we have some code for which
it appears we are not going to get a relicensing agreement.  So at this
point it's either remove it or replace it.

Thanks to Matthew publishing the history tools I've started to try to
help out with this and have quickly learned that I really don't know
what to do.  I'm going to present an example here and I'm asking you for
your considered opinion.

SequenceableCollection>>asStringWithCr

This method goes back to Squeak1.0 and has 5 versions leading up to what
is today in Squeak3.10.2-7197/Squeak4.0alpha.  It is the second version
by Douglas McPherson that has the problem (we have not received a
relicensing agreement from Douglas).  I'm going to paste in, oldest to
newest, all versions so we can discuss this concretely.  In each I will
number the version in front of the containing filename for discussion
convenience and mark changed lines (relative to the earlier version)
with a + in the first column.  The filenames used are those as
distributed with the Matthew/Yoshiki history tools; sometimes these
don't directly map to the original updates (think updates that load
MCZs).

1) SqueakV1.sources:

asStringWithCr
	"Convert to a string with returns between items.  Elements are usually
strings.
	 Useful for labels for PopUpMenus."
	| labelStream |
	labelStream _ WriteStream on: (String new: 200).
	self do: [:each |
		(each isKindOf: String)
			ifTrue: [labelStream nextPutAll: each; cr]
			ifFalse: [each printOn: labelStream; cr]].
	self size > 0 ifTrue: [labelStream skip: -1].
	^ labelStream contents!

2) 442TweaksForBeta-di.cs:

!SequenceableCollection methodsFor: 'converting' stamp: 'djm 11/20/1998
05:44'!
asStringWithCr
	"Convert to a string with returns between items.  Elements are
usually strings.
	 Useful for labels for PopUpMenus."
	| labelStream |
	labelStream _ WriteStream on: (String new: 200).
	self do: [:each |
		(each isKindOf: String)
			ifTrue: [labelStream nextPutAll: each; cr]
+			ifFalse: [each printOn: labelStream. labelStream cr]].
	self size > 0 ifTrue: [labelStream skip: -1].
	^ labelStream contents! !

3) 5994-004-systemMod.cs:

!SequenceableCollection methodsFor: 'converting' stamp: 'yo 8/28/2002
15:39'!
asStringWithCr
	"Convert to a string with returns between items.  Elements are
usually strings.
	 Useful for labels for PopUpMenus."
	| labelStream |
	labelStream _ WriteStream on: (String new: 200).
	self do: [:each |
+		(each isKindOf: AbstractString)
			ifTrue: [labelStream nextPutAll: each; cr]
			ifFalse: [each printOn: labelStream. labelStream cr]].
	self size > 0 ifTrue: [labelStream skip: -1].
	^ labelStream contents! !

4) 6651ExternalCleanup.cs:

!SequenceableCollection methodsFor: 'converting' stamp: 'ar 4/10/2005
18:02'!
asStringWithCr
	"Convert to a string with returns between items.  Elements are
usually strings.
	 Useful for labels for PopUpMenus."
	| labelStream |
	labelStream _ WriteStream on: (String new: 200).
	self do: [:each |
+		each isString
			ifTrue: [labelStream nextPutAll: each; cr]
			ifFalse: [each printOn: labelStream. labelStream cr]].
	self size > 0 ifTrue: [labelStream skip: -1].
	^ labelStream contents! !

5) Squeak39g-7056+3102-7179.changes:

!SequenceableCollection methodsFor: 'converting' stamp: 'ar 4/10/2005
18:02' prior: 29748463!
asStringWithCr
	"Convert to a string with returns between items.  Elements are
usually strings.
	 Useful for labels for PopUpMenus."
	| labelStream |
+	labelStream := WriteStream on: (String new: 200).
	self do: [:each |
		each isString
			ifTrue: [labelStream nextPutAll: each; cr]
			ifFalse: [each printOn: labelStream. labelStream cr]].
	self size > 0 ifTrue: [labelStream skip: -1].
	^ labelStream contents! !

Now as I said above the problem with with version 2.  As you can see
each of these changes is at most one line and by and large frankly
rather obvious.  The change in 2 is clearly a bug fix and the
straightforward solution.

So the question is what do I do with this method, which is currently
distributed as version 5, so that it does not infringe on djm's rights
to maintain version 2 as SqueakL?  Admittedly it's unlikely that djm
actually wants this right, but lacking a clear statement to the
contrary, it's the default we have to assume.

How much of a change is a significant change?

Should I delete the current implementation entirely and rewrite it from
scratch?  If so, how much am I allowed to know about the prior
implementations?

Ken
  

Ken Causey wrote:

This might be a nutty idea but...

What if the code that finds the methods that are badly licensed and that
we have no means of relicensing with a better license is simply removed
by the finding code. Then the development teams or whomever is using the
development version fixes this in the debugger when code is run that
uses the missing methods.

This way there is nobody looking at implementations to write tests or
documentation. The code is mechanically removed, no implementation
details revealed other than an understanding of what the calling code is
expecting.

That makes it as clean a break from the past as is possible. It will
never be demonstrable that an implementer did not peak in an earlier
image. But we will have done all that could be done to do this cleanly
with integrity to the process. That is all we can ask or do.

Don't know if this helps. But I did want to toss it out there in the
arena of ideas. :)

Jimmie

Hi Ken,

How many methods are we talking about here? How many are 1 line, 2 line,
have branches, etc? How many entire classes? Do we have some tools to
work this out?

For simple 1 line methods, I think it's reasonable for our policy to be
do nothing. This should certainly be the case for accessors, lazy
initializers and other stuff we deem and can later justify as common place.

For long/complex methods, I think the answer might be to refactor these
into new smaller methods. Slight of hand perhaps, but I think it gets
the job done.

After dealing with the 1 liners and long/complex methods, how many are
we left with?

I suspect the only way to deal with the remainder is for us to write
unit tests, comments and then hire someone who has never touched Squeak
before to write the methods.  They will have to convince the board and
law foundation foundation that they have not touched Squeak before.
It'll never be perfect but I think perfect is impossible.

- Zulq

Ken Causey wrote: