Add support for VersionVault

Review Request #11582 — Created April 7, 2021 and updated

david
RBTools
master
rbtools

We will soon be adding a new SCMTool implementation to Power Pack to connect to
HCL VersionVault and IBM ClearCase, supplanting the old community-driven
ClearCase support. This change adds RBTools-side support for this.

This is based on code delivered to us in partnership with HCL, but has had
significant changes. The largest change is that instead of having a completely
separate SCMClient implementation in RBTools, this is built into the same
ClearCase client. Most of the code paths are the same between the two types of
backends, and the major difference is that the diff output for VersionVault
includes additional information in the headers (the VOB OID for each changed
element, and a JSON-encoded blob to drop into the FileDiff's extra_data in order
to do better post-processing on the diff display).

  • Posted changes from the current checkout (rbt post with no args) including
    new, modified, deleted, and binary files.
  • Posted changes that spanned multiple VOBs.
  • Posted a change that spanned multiple VOBs, including one that was not
    configured as part of the server-side repository. Saw expected
    warning.
  • Posted changes for UCM activities for both a single and multiple
    revisions.

More testing to come.

Summary
Add support for VersionVault
Description From Last Updated

E501 line too long (81 > 79 characters)

reviewbotreviewbot

E722 do not use bare except'

reviewbotreviewbot

, optional?

chipx86chipx86

F401 'rbtools.api.errors.AuthorizationError' imported but unused

reviewbotreviewbot

Blank line after class docstring.

chipx86chipx86

These are so close. Can we build up the cmdline based on self.label and then pass in to execute()?

chipx86chipx86

Should be is_ucm.

chipx86chipx86

Can you specify explicitly which is which?

chipx86chipx86

Can you update this to point to the full class path?

chipx86chipx86

E501 line too long (81 > 79 characters)

reviewbotreviewbot

I think we can just use os.pathsep for this, which is for things like $PATH. Same with the earlier usage ...

chipx86chipx86

Is Qp part of this?

chipx86chipx86

These are pretty close. Can we build up a cmdline and then pass it to execute()?

chipx86chipx86

F821 undefined name 're'

reviewbotreviewbot

Let's compile the regex right before and then reuse that. I don't think we need the [ and ] though.

chipx86chipx86

You can pass a tuple of choices here.

chipx86chipx86

You can use os.pathsep here too. Kinda wonder if we should predefine constants for the environment variables on the class ...

chipx86chipx86

Given how simple this logic is, how about just: if label != 'LATEST' and not self._is_a_label(label, tag): labels_present = False

chipx86chipx86

Small nit, but mind alphabetizing these? We also build the string twice. Might as well do it once and then ...

chipx86chipx86

We can just pass split_lines=True to execute().

chipx86chipx86

F821 undefined name 're'

reviewbotreviewbot

Just to check, are we splitting on \s, :, or @? Or on \s:@? The [] will give us the ...

chipx86chipx86

Instead of setting twice, maybe have the second do: extra_data['new'] = extra_data['old'].copy()

chipx86chipx86

No need for , None on these.

chipx86chipx86

E722 do not use bare except'

reviewbotreviewbot

E201 whitespace after '{'

reviewbotreviewbot

E202 whitespace before '}'

reviewbotreviewbot

E272 multiple spaces before keyword

reviewbotreviewbot

E272 multiple spaces before keyword

reviewbotreviewbot

F841 local variable 'CLEARCASE_PN' is assigned to but never used

reviewbotreviewbot

F841 local variable 'commmand' is assigned to but never used

reviewbotreviewbot

F821 undefined name 'command'

reviewbotreviewbot

E501 line too long (91 > 79 characters)

reviewbotreviewbot

E501 line too long (97 > 79 characters)

reviewbotreviewbot

E501 line too long (97 > 79 characters)

reviewbotreviewbot

E501 line too long (109 > 79 characters)

reviewbotreviewbot

E501 line too long (107 > 79 characters)

reviewbotreviewbot

E501 line too long (115 > 79 characters)

reviewbotreviewbot

E201 whitespace after '{'

reviewbotreviewbot

E202 whitespace before '}'

reviewbotreviewbot

E501 line too long (91 > 79 characters)

reviewbotreviewbot

E501 line too long (97 > 79 characters)

reviewbotreviewbot

E501 line too long (97 > 79 characters)

reviewbotreviewbot

E501 line too long (107 > 79 characters)

reviewbotreviewbot

E201 whitespace after '{'

reviewbotreviewbot

E202 whitespace before '}'

reviewbotreviewbot

F811 redefinition of unused 'test_get_repository_info_dynamic' from line 107

reviewbotreviewbot

E501 line too long (84 > 79 characters)

reviewbotreviewbot

E501 line too long (91 > 79 characters)

reviewbotreviewbot

E501 line too long (97 > 79 characters)

reviewbotreviewbot

E501 line too long (97 > 79 characters)

reviewbotreviewbot

E501 line too long (107 > 79 characters)

reviewbotreviewbot

E201 whitespace after '{'

reviewbotreviewbot

E202 whitespace before '}'

reviewbotreviewbot

E501 line too long (84 > 79 characters)

reviewbotreviewbot

E501 line too long (91 > 79 characters)

reviewbotreviewbot

E501 line too long (97 > 79 characters)

reviewbotreviewbot

E501 line too long (97 > 79 characters)

reviewbotreviewbot

E501 line too long (107 > 79 characters)

reviewbotreviewbot

E131 continuation line unaligned for hanging indent

reviewbotreviewbot

E201 whitespace after '{'

reviewbotreviewbot

E202 whitespace before '}'

reviewbotreviewbot

Probably should be , optional.

chipx86chipx86

The common part was split out, but these are overriding instead of appending to it.

chipx86chipx86

No blank line here.

chipx86chipx86

Typo: "becaue" -> "because"

chipx86chipx86

To ensure doc building works right, the format: should be format::. This will turn this into a code block.

chipx86chipx86

I have no idea if it's faster or not, but we can probably just have this be: if '%s/' % ...

chipx86chipx86

Is "base clearcase" correct? Drawing a blank on the term. Is this just IBM ClearCase?

chipx86chipx86

Can we update these to use the formal capitalization of these names?

chipx86chipx86

Small nit: Alphabetical order?

chipx86chipx86

E303 too many blank lines (2)

reviewbotreviewbot

E501 line too long (89 > 79 characters)

reviewbotreviewbot
Checks run (1 failed, 1 succeeded)
flake8 failed.
JSHint passed.

flake8

david
Review request changed

Summary:

-WIP integrating ClearCase/VersionVault
+[WIP] Add support for VersionVault

Description:

~  

WIP integrating ClearCase/VersionVault

  ~

We will soon be adding a new SCMTool implementation to Power Pack to connect to

  + HCL VersionVault and IBM ClearCase, supplanting the old community-driven
  + ClearCase support. This change adds RBTools-side support for this.

  +
  +

This is based on code delivered to us in partnership with HCL, but has had

  + significant changes. The largest change is that instead of having a completely
  + separate SCMClient implementation in RBTools, this is built into the same
  + ClearCase client. Most of the code paths are the same between the two types of
  + backends, and the major difference is that the diff output for VersionVault
  + includes additional information in the headers (the VOB OID for each changed
  + element, and a JSON-encoded blob to drop into the FileDiff's extra_data in order
  + to do better post-processing on the diff display).

  +
  +

There's still significant testing (manual and automatic) and documentation work

  + to do on this, hence the WIP.

Commits:

Summary
-
WIP integrating ClearCase/VersionVault
+
[WIP] Add support for VersionVault

Diff:

Revision 2 (+1452 -452)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

chipx86
  1. 
      
  2. rbtools/clients/__init__.py (Diff revision 2)
     
     

    , optional?

  3. rbtools/clients/clearcase.py (Diff revision 2)
     
     
     

    Blank line after class docstring.

  4. rbtools/clients/clearcase.py (Diff revision 2)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     

    These are so close. Can we build up the cmdline based on self.label and then pass in to execute()?

  5. rbtools/clients/clearcase.py (Diff revision 2)
     
     

    Should be is_ucm.

  6. rbtools/clients/clearcase.py (Diff revision 2)
     
     
     
     
     
     

    Can you specify explicitly which is which?

  7. rbtools/clients/clearcase.py (Diff revision 2)
     
     

    Can you update this to point to the full class path?

  8. rbtools/clients/clearcase.py (Diff revision 2)
     
     
     
     
     

    I think we can just use os.pathsep for this, which is for things like $PATH.

    Same with the earlier usage as well.

    1. I suppose it's possible that would work, but I think this handles the common case of cygwin, where os.pathsep is ":" but cleartool will be expecting ";".

  9. rbtools/clients/clearcase.py (Diff revision 2)
     
     

    Is Qp part of this?

    1. That's part of the code we got from HCL. I'm not 100% sure one way or the other yet, since my UCM knowledge is still pretty spare.

    2. Answer: Qp is part of it.

  10. rbtools/clients/clearcase.py (Diff revision 2)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     

    These are pretty close. Can we build up a cmdline and then pass it to execute()?

  11. rbtools/clients/clearcase.py (Diff revision 2)
     
     

    Let's compile the regex right before and then reuse that.

    I don't think we need the [ and ] though.

  12. rbtools/clients/clearcase.py (Diff revision 2)
     
     

    You can pass a tuple of choices here.

  13. rbtools/clients/clearcase.py (Diff revision 2)
     
     
     
     
     

    You can use os.pathsep here too.

    Kinda wonder if we should predefine constants for the environment variables on the class on first use or something, cut down on the conditionals further. Super not important, but just a food-for-thought thing.

  14. rbtools/clients/clearcase.py (Diff revision 2)
     
     
     
     
     
     

    Given how simple this logic is, how about just:

    if label != 'LATEST' and not self._is_a_label(label, tag):
        labels_present = False
    
  15. rbtools/clients/clearcase.py (Diff revision 2)
     
     
     

    Small nit, but mind alphabetizing these?

    We also build the string twice. Might as well do it once and then set.

  16. rbtools/clients/clearcase.py (Diff revision 2)
     
     
     
     
     
     
     
     
     
     
     

    We can just pass split_lines=True to execute().

  17. rbtools/clients/clearcase.py (Diff revision 2)
     
     

    Just to check, are we splitting on \s, :, or @? Or on \s:@? The [] will give us the former.

    1. I don't know the answer to this yet.

  18. rbtools/clients/clearcase.py (Diff revision 2)
     
     
     
     
     
     
     
     
     

    Instead of setting twice, maybe have the second do:

    extra_data['new'] = extra_data['old'].copy()
    
    1. These aren't the same.

  19. rbtools/clients/clearcase.py (Diff revision 2)
     
     

    No need for , None on these.

  20. 
      
david
Review request changed

Change Summary:

Continued progress

Commits:

Summary
-
[WIP] Add support for VersionVault
+
Add support for VersionVault

Diff:

Revision 3 (+2204 -472)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

david
Review request changed

Change Summary:

Review Bot

Commits:

Summary
-
Add support for VersionVault
+
Add support for VersionVault

Diff:

Revision 4 (+2300 -472)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

david
david
david
chipx86
  1. This is looking great. A few small things.

  2. rbtools/clients/__init__.py (Diff revision 7)
     
     

    Probably should be , optional.

  3. rbtools/clients/clearcase.py (Diff revision 7)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     

    The common part was split out, but these are overriding instead of appending to it.

  4. rbtools/clients/clearcase.py (Diff revision 7)
     
     
     
     

    No blank line here.

  5. rbtools/clients/clearcase.py (Diff revision 7)
     
     

    Typo: "becaue" -> "because"

  6. rbtools/clients/clearcase.py (Diff revision 7)
     
     
     
     
     
     
     
     

    To ensure doc building works right, the format: should be format::. This will turn this into a code block.

  7. rbtools/clients/clearcase.py (Diff revision 7)
     
     

    I have no idea if it's faster or not, but we can probably just have this be:

    if '%s/' % tag in path:
        break
    
  8. rbtools/clients/clearcase.py (Diff revision 7)
     
     
     
     
     
     
     
     
     
     
     

    Purely a suggestion, and not important for this change: Might be nice to have a _run_cleartool() command or something that takes the vobs or other things we need and builds them in a platform-dependent way in one place, so we can consolidate this sort of logic.

    The environment variable constants we define elsewhere could maybe be done on a class level.

  9. rbtools/clients/tests/test_clearcase.py (Diff revision 7)
     
     

    Is "base clearcase" correct? Drawing a blank on the term. Is this just IBM ClearCase?

    1. Yeah, it's correct. The distinction here is base clearcase vs. UCM

  10. rbtools/clients/tests/test_clearcase.py (Diff revision 7)
     
     

    Can we update these to use the formal capitalization of these names?

  11. rbtools/commands/diff.py (Diff revision 7)
     
     
     

    Small nit: Alphabetical order?

  12. 
      
david
Review request changed

Change Summary:

  • Add in documentation for various posting modes.
  • Fix for when /main/1 has been rmvered.
  • Fix for finding the correct predecessor version in a UCM activity when revisions have been rmvered.
  • Fix up various comments and other minor things.

Summary:

-[WIP] Add support for VersionVault
+Add support for VersionVault

Description:

   

We will soon be adding a new SCMTool implementation to Power Pack to connect to

    HCL VersionVault and IBM ClearCase, supplanting the old community-driven
    ClearCase support. This change adds RBTools-side support for this.

   
   

This is based on code delivered to us in partnership with HCL, but has had

    significant changes. The largest change is that instead of having a completely
    separate SCMClient implementation in RBTools, this is built into the same
    ClearCase client. Most of the code paths are the same between the two types of
    backends, and the major difference is that the diff output for VersionVault
    includes additional information in the headers (the VOB OID for each changed
    element, and a JSON-encoded blob to drop into the FileDiff's extra_data in order
    to do better post-processing on the diff display).

-  
-  

There's still significant testing (manual and automatic) and documentation work

-   to do on this, hence the WIP.

Testing Done:

  +
  • Posted changes from the current checkout (rbt post with no args) including
    new, modified, deleted, and binary files.
  +
  • Posted changes that spanned multiple VOBs.
  +
  • Posted a change that spanned multiple VOBs, including one that was not
    configured as part of the server-side repository. Saw expected
    warning.
  +
  • Posted changes for UCM activities for both a single and multiple
    revisions.
  +
  +

More testing to come.

Commits:

Summary
-
Add support for VersionVault
+
Add support for VersionVault

Diff:

Revision 8 (+2776 -600)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

david
Review request changed

Checks run (2 succeeded)

flake8 passed.
JSHint passed.
Loading...