repair.strip and backups

[Nicolas Dumazet]

Hello!

I am looking at various strip calls, and was wondering about this
backup= parameter, which affects bundle creation and deletion in
repair.strip.

== Which bundles? ==
There are two bundles that can be created during a strip:
1) a "full bundle" (user-oriented) which contains the stripped revisions
2) a "partial bundle" (but more exactly "non-descendants" bundle),
   which holds the revisions > strippedrevs but that are not descendants
   of the stripped node. This one is temporary and used to restore
   changesets after changelog truncation.

Full bundle is created, or not.
Partial bundle is always created if necessary, but not always deleted
after a successful strip.

== Behavior depending on 'backup' value ==
As far as I can see, there are 3 possible values for the backup
parameter:
- "all" (default): 
  * full: create
  * partial: delete
- "strip":
  * full: dont create
  * partial: keep
- anything else (None, "none", ...):
  * full: dont create
  * partial: delete


My first remark would be that I dislike this three-valued
"backup" parameter. In particular,
  repair.strip(..., backup='strip')
is meaningless to a candid eye. I would like to use instead two
booleans, maybe (backup=False, keeptmp=True), for a clearer semantic.
Any remarks on this?

== Usage ==

mq.py:
  - qpop. STRIP
  - qrefresh (strip the applied revision before committing the newer one). STRIP
  - qclone. "stripping applied patches from destination repository". NONE
  - strip:
    * default: ALL
    * -b (deprecated): STRIP
    * --nobackup: NONE
  (and two deprecated usages:)
  - qrestore. (not sure why). STRIP
  - mq.mergeone, "# apply failed, strip away that rev and merge". Only
    used in qpush -m/--merge. STRIP 
rebase.py:
  - abort, "# no backup of rebased cset versions needed". ALL
  - rebase, "# backup the old csets by default". ALL

Everytime that STRIP mode is used, the "partial bundle" is kept. Why is
that necessary? A qrefresh seems quite harmless, and keeping this
partial bundle on success does not sound necessary. When popping
patches, keeping the partial bundle after a successful pop does not
sound useful to anyone either.

Is there is a use case to keep those partial bundles?
I grepped the tests for a "strip failed, partial bundle stored in "
message, and could not find one. Maybe it's time to get rid of this odd
STRIP mode?

== Compression ==

repair._bundle uses by default BZ2 compression.
For "full bundles", it makes sense, as those are likely to be kept
around. But partial bundles are temporary, merely an internal detail.
Compressing takes time when bundling, and needs time when unbundling.

I think that in ALL and NONE cases, where temp bundles are not kept, we
could get rid of compression, speeding up the related commands.
I have a patch doing this. Any comments?


Thank you for reading ;)

-- 
Nicolas Dumazet — NicDumZ