Re: Aw: News spin of repository conversion

[Daniel J S. <dan...@ie...>]

On 10/21/2017 06:42 AM, Eric S. Raymond wrote:
> Daniel J Sebald <dan...@ie...>:
>> I'd say no manual correction for that.  The pool to search through is all
>> entries, 6000+.  I was thinking the only manual adjustment might be the
>> substitution of "empty log message" as one of the last steps. That's only
>> 300 entries, much more reasonable to walk through.
> 
> Agreed.
> 
> You might want to take alook at the logic, though.  I *think* my algorithm
> is equivalent to yours, but I could be wrong.

Close, but I believe there is one small detail different.  I see now 
this could have been done as just a single loop--hindsight--but that's 
sort of too much conditional code and it's easier to understand as two 
loops.


> For each changeset in the repository conversion that includes a ChangeLog blob:
> 
> 1. Fetch the ancestral version of ChangeLog, if it has one.
> Compare leading lines until you hit a pair that doesn't match.
> That line number beciomes the value of 'changeline'. If you
> find no ancestor, let changeline be 0.
> 
> 2. Walk through the (newer) Changelog line file. Parse each attribution
> time for date and email address, soeing them in variables 'date' and 'addr'.
> 
> 3. Break out of that loop when (a) the line you just parsed is at or
> after changeline, or (b) you reach EOF.

You were thinking about this correctly, but the code is a subtle 
difference in the fact that it "continues" out of the loop search when 
the line in question does not match author info.  By using such a 
construct, the test for n < changeline does not happen unless the line 
in question meets th criteria of being an author line, i.e., notice in 
the following that the change in flow is always a "continue".

     if ',' in line:
         # Multiple attributions...ignore for now
         continue
     # Deal with some address masking
     line = line.replace(" <at> ", "@")
     space = line.find(" ")
     if space < 0:
         continue
     date = line[:space]
     # Avoid fatal error in the attribution builder
     try:
         time.strptime(date, "%Y-%m-%d")
     except ValueError:
         continue
     addr = line[space+1:]
     if len(date) < 27:
         # Try to never assign the same generated
         # timestamp.  This may help later on when
         # we need action stamps to be unique, e.g.
         # for mailbox_in.
         for offset in range(60):
             fdate = date + "T12:00:%02dZ" % offset
             if fdate in generated:
                 continue
             generated.add(fdate)
             date = fdate
             break
         else:
             raise Recoverable("reposurgeon: can't uniquify ChangeLog 
date.")
     # Placement of this statement is very important.
     # The effect we want is for all lines before the
     # first changed one to be parsed for date and
     # addr, but for only the *last* date and addr
     # to be kept/  This handles the case where the
     # changes secrtion begins inside an entry (after
     # its header line) as well as the obviuos case
     # where the changed section *begins* with an
     # attribution line.
     if n < changeline:
         continue

Hence, what is really happening above is a search for the first author 
line that comes at or after 'changeline'.  That's not correct.

 From step 3, I think you meant to place the test of 'n' and 
'changeline' at the front of loop and 'break' (on >) instead.  I.e.,

     if n > changeline:
         break
     if ',' in line:
         # Multiple attributions...ignore for now
         continue
     # Deal with some address masking
     line = line.replace(" <at> ", "@")
     space = line.find(" ")
     if space < 0:
         continue
ETC.

It should break if "n > changeline", not if "n >= changeline", because 
we want to include the scenario of author info at the first changed line 
(which is the most typical scenario).

While on the subject, I see you've include the "if only ChangeLog 
changes, ignore" heuristic.  What about the case of the change in the 
ChangeLog being a subtraction rather than an addition.  I left that out 
because all the scenarios I imagined that happening were a situation we 
wanted to ignore any authorship change and just let the committer have 
authorship (e.g., wholesale swap of ChangeLog.0 to ChangeLog.1, some 
typo in the authorship line was corrected).  I'm not sure your code 
differentiates between the two (it looks to be searching just for 
change), but really this is a very low likelihood of occurrence so maybe 
it isn't worth toiling over that.

Dan