RE: [PATCH] New SymbolTransforms

Hi,

Michael Haggerty wrote:
> > * IgnoreTagsExceptSpecifiedOnesSymbolTransform
> > * ExcludeBranchesSymbolTransform
>
> I'm not sure what you want me to do with these last two
> SymbolTransforms. 

Short version: They're no longer needed.

Long version: Now we have TagOnlyTransform and BranchOnlyTransform,
those other two can be implemented by carefully nesting other
SymbolTransforms, which can easily be done inside the options.py
file.  There's no need to complicate cvs2svn by adding them to the
core.

FYI only, NOT for inclusion, the options.py code is:

  def ignoreTagsExceptSpecifiedOnesSymbolTransform(tags_to_keep):
    if not tags_to_keep:
        return TagOnlyTransform(IgnoreSymbolTransform('.*'))

    # Create a regexp that matches anything EXCEPT the literal
    # strings in the passed list.  This doesn't work if the
    # list is empty (hence the special case above).
    #
    # First we generate a regexp that matches JUST the literal
    # strings in the passed list, ...
    r = '|'.join(['(?:' + re.escape(tag) + '$)'
                  for tag in tags_to_keep])
    # ... then we negate it.
    r = '(?!' + r + ').*'

    return TagOnlyTransform(IgnoreSymbolTransform(r))

  def ignoreBranchesSymbolTransform():
    return BranchOnlyTransform(
        RegexpSymbolTransform('(.*)', 'cvs2svn-excluded-br-\\1'))

  def ignoreBranchesInFolderSymbolTransform(cvs_path):
    return SubtreeSymbolTransform(cvs_path,
                ignoreBranchesSymbolTransform())

> It seems to me that the same functionality can be
> implemented via SymbolStrategyRules, which are generally
> more powerful and less fidgety.  Is there a reason to
> implement them as SymbolTransforms?

We have certain folders in CVS that have a "never branch" policy.
The ignoreBranchesInFolderSymbolTransform() allows me to remove
the few branches that were accidentally created in these folders.
I.e. someone checked out a/, b/ and c/ then did "cvs tag -b
FOO-BR ." to branch everything, but they should've only branched a/
and b/.  I need to keep FOO-BR in a/ and b/, but exclude it in c/.
This can't be done with SymbolStrategyRules alone.

I believe the tag-exclusion rule was for the (now-dropped) exclude
vendor branches patch.  With early versions of that patch, there
would be an error if the vendor release tags weren't excluded by a
SymbolTransform.  Since that patch has been dropped, there's no
longer any need for the tag-exclusion rule, and I'll remove that
from my options.py file.  As you say, it can be done with
SymbolStrategyRules.  (And I'm already specifying the right
SymbolStrategyRules, so this change will simplify my options.py
and remove duplication - thanks!)

>> P.S. It looks like SymbolTransforms have to be defined in the
>> cvs2svn core - I originally tried to define these in my
>> options.py file, but that fails with a pickle error.
>
> Oh, yes, that's awkward.  The reason is that the options file
> is execfile()ed rather than imported, so the Pickle module has
> no way to know how to access the class definitions when it is
> trying to unpickle the SymbolTransform instances.
>
> [...]
>
> It would be nice if this were easier.  But off the top of my head
> I can't think of a way.

My thoughts on seeing this problem were along the lines of "why is
cvs2svn pickling the settings from the options file, rather than
just reading the options file each time?"  Unfortunately, I doubt
I'll get time to fix this.

In another e-mail, Michael Haggerty wrote:
> > 3) Symbol strategy changes.  Did you have any comments about the
> > "Default-exclude symbol strategy" patch that I sent?
> 
> I gave some feedback on that patch in an email on March 5 [1].
> You were probably confused because the response was so prompt :-)
> 
> Let me know how you would like to proceed.

Ahh.  Sorry about missing that.  I'll reply to it separately,
this e-mail is getting rather long.  (And I suspect I'm going to
need to attach a tested patch to my reply, which will take a
while).

> I also still have:
>
> 4) Remove no-op revisions
>
> in my local git repository.  Should I infer that you now think
> that this change is no longer worthwhile?

It's not necessary for my particular cvs2svn conversion, as
renumber_branch.py does what I need.  So I can't justify spending
more work time on it, I'm afraid.  If you or anyone else think
it's valuable, then feel free to finish it off and incorporate it.
Otherwise it can be dropped.  (I got the impression that you were
pretty ambivalent about this patch, anyway).

Kind regards,

Jon



**********************************************************************
This email and its attachments may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Cabot Communications Ltd.

If you are not the intended recipient of this email and its attachments, you must take no action based upon them, nor must you copy or show them to anyone.

Cabot Communications Limited
Verona House, Filwood Road, Bristol BS16 3RY, UK
+44 (0) 1179584232

Co. Registered in England number 02817269

Please contact the sender if you believe you have received this email in error.

**********************************************************************


______________________________________________________________________
This email has been scanned by the MessageLabs Email Security System.
For more information please visit http://www.messagelabs.com/email 
______________________________________________________________________

------------------------------------------------------
http://cvs2svn.tigris.org/ds/viewMessage.do?dsForumId=1667&dsMessageId=2466663

To unsubscribe from this discussion, e-mail: [[email protected]].