bup-init man page comments

[Greg Troxel]

As part of trying to debug problems with saving to a main host from
0.33.10, I went to create a new repo to bup get --rewrite to, and
realized I was unclear on how to configure hashsplit, because my
bup-snapshot package lacks my hacky patches to force hashsplit to 16
instead of 13 -- and I can now skip that! I had some confusion.


1) bup-init.1 says that it "initializes" a repository. There is no
suggestion that running it again on an existing repository is safe or
does anything useful. With a POSIX language lawyer hat on, I see the
omission as mostly implying UB. I suggest adding

bup init can be run safely on an existing repository. It will not
modify any data. It will add configuration that a fresh init would
set, but which is lacking in the existing repository. For now, this
is setting "bup.repo.id" to a fresh random value.

2) While there is a see also to bup-config.5, there's no hint or
suggestion on how to set bup.split.files. It doesn't seem to default to
setting 16 on new repos. I didn't find a way to do this, other than
hand editing config, and surely git config would be fine too.


3) There's very confusing section about remote repositories:

OPTIONS
−r, −−remote=[user@]host:[path], −−remote=URL
Initialize not only the local repository, but also the specified
remote repository. This is not necessary if you intend to use
the default location on the server (ie. with no path). By
default the connection to the remote server is made with SSH.
See bup(1) REMOTE OPTIONS for further information.


I have multiple questions about this:

a) Why does bup init, when using -r, touch *at all* or require
anything about a local repo? If I'm initializing a remote, shouldn't
it just do that?

b) Surely I could run bup init on the remote machine. That seems
better and less complicated. But I can see how having -r is useful,
if the access to the remote is via bup only, with no regular shell
access -- but that calls into question the point of that restriction.

c) Not necessary for "default location" does not make sense. "no
path" is confusing, because surely that means with the default path,
but maybe an explicit .bup and a missing path have different
semantics. IMHO, the right way is either bup save needs an
initialized repo, or it doesn't. Having bup save be willing to
initialize ~/.bup if path is not given, but not path, seems like too
much magic. I vote for no implicit initialization.

d) the default about SSH seems outdated; I suggest deleting the
sentence as the "See bup(1) REMOTE OPERATIONS" is sufficient and
doesn't risk outdated/conflicting information. (Plus SSH in caps is
unusual.)

[Rob Browning]

Greg Troxel <g...@lexort.com> writes:

> 1) bup-init.1 says that it "initializes" a repository. There is no
> suggestion that running it again on an existing repository is safe or
> does anything useful. With a POSIX language lawyer hat on, I see the
> omission as mostly implying UB.

That's a reasonable interpretation, i.e. more is not formally promised
yet. Though in practice, since the basis is git-init, it establishes
some of the behavior (we do whatever it does), and we also mimic some of
its semantics for bup-specific bits.

Roughly (in main right now), we call git-init (which will create or
refresh the repo), and then among other things, do this:

# Always set the indexVersion so bup works with any git version
if refresh:
if get_config(b'bup.repo.id') is None:
set_config(b'bup.repo.id', make_repo_id())
if get_config(b'pack.indexVersion', opttype='int') != 2:
set_config(b'pack.indexVersion', b'2')
else: # "no repo" (see above), reestablish defaults (as git does)
set_config(b'bup.repo.id', make_repo_id())
set_config(b'core.logAllRefUpdates', b'true') # enable the reflog
set_config(b'pack.indexVersion', b'2')

We detect whether we're "refreshing" the same way git does (or did ---
thanks to Johannes for tracking that down), by looking for repo/HEAD.

Note that we preserve an existing bup.repo.id.

> 2) While there is a see also to bup-config.5, there's no hint or
> suggestion on how to set bup.split.files. It doesn't seem to default to
> setting 16 on new repos.

Right, for now legacy:13 is still the default, but as mentioned in the
pending release notes (note/main.md):

* The deduplication granularity can now be changed by a new
`bup.split.files` configuration option which defaults to `legacy:13`
(the current behavior), but should probably be set to a higher value
like `legacy:16` in new repositories (say via `git-config --file
REPO/config bup.split.files legacy:16`). The default for new
repositories will eventually be raised. See `bup-config`(5) for
additional information.

> I didn't find a way to do this, other than hand editing config, and
> surely git config would be fine too.

bup.split.files is just a git config values, so you set it with
git-config, and it's documented in bup-config(5) (guessing you found
that). Though we should mention git-config(1) and bup-config(5) in
bup-init(1).

For example:

git --git-dir="$BUP_DIR" config bup.split.files legacy:16
git --git-dir="$BUP_DIR" config bup.split.trees true

> a) Why does bup init, when using -r, touch *at all* or require
> anything about a local repo? If I'm initializing a remote, shouldn't
> it just do that?

If I recall correctly, this was added to just clearly document
long-standing behavior (related to the original/previous emphasis on
~/.bup). I suspect that's not what we'd choose now.

> willing to initialize ~/.bup if path is not given, but not path,
> seems like too much magic. I vote for no implicit initialization.

I tend to agree, were we starting from scratch, but that would be a
notable break in backward compatibility.

> d) the default about SSH seems outdated; I suggest deleting the
> sentence as the "See bup(1) REMOTE OPERATIONS" is sufficient and
> doesn't risk outdated/conflicting information. (Plus SSH in caps is
> unusual.)

I'm not sure I follow. I think this is still just referring to ~/.bup,
which would be automatically created in the past, though now that I
think about it, we actually have changed some of that, so I should
double-check the current situation...

Thanks
--
Rob Browning
rlb @defaultvalue.org and @debian.org
GPG as of 2011-07-10 E6A9 DA3C C9FD 1FF8 C676 D2C4 C0F0 39E9 ED1B 597A
GPG as of 2002-11-03 14DD 432F AE39 534D B592 F9A0 25C8 D377 8C7E 73A4

[Greg Troxel]

Rob Browning <r...@defaultvalue.org> writes:

> Greg Troxel <g...@lexort.com> writes:
>
>> 1) bup-init.1 says that it "initializes" a repository. There is no
>> suggestion that running it again on an existing repository is safe or
>> does anything useful. With a POSIX language lawyer hat on, I see the
>> omission as mostly implying UB.
>
> That's a reasonable interpretation, i.e. more is not formally promised
> yet. Though in practice, since the basis is git-init, it establishes
> some of the behavior (we do whatever it does), and we also mimic some of
> its semantics for bup-specific bits.

I guess I should have said more coherently: if we mean, as I think we
do, that it is safe and reasonable to call bup init on an already
initialized repository, then it would be good to simply say that. I
would expect many readers to think it scary to call init on a place with
existing data.

> Roughly (in main right now), we call git-init (which will create or
> refresh the repo), and then among other things, do this:
>
> # Always set the indexVersion so bup works with any git version
> if refresh:
> if get_config(b'bup.repo.id') is None:
> set_config(b'bup.repo.id', make_repo_id())
> if get_config(b'pack.indexVersion', opttype='int') != 2:
> set_config(b'pack.indexVersion', b'2')
> else: # "no repo" (see above), reestablish defaults (as git does)
> set_config(b'bup.repo.id', make_repo_id())
> set_config(b'core.logAllRefUpdates', b'true') # enable the reflog
> set_config(b'pack.indexVersion', b'2')
>
> We detect whether we're "refreshing" the same way git does (or did ---
> thanks to Johannes for tracking that down), by looking for repo/HEAD.
>
> Note that we preserve an existing bup.repo.id.

yes, that seems implied by "bup init on existing is safe".

Interesting about upgrading pack.indexVersion.

>> 2) While there is a see also to bup-config.5, there's no hint or
>> suggestion on how to set bup.split.files. It doesn't seem to default to
>> setting 16 on new repos.

It is not fair, but I was seeing this as a bit of a regression, because
I'm moving from hand-patched to 16 to trying to use the new mechanism.
Now, I'm not finding the bup-config xref.

Perhaps add

See bup-config(5) for a description of configuration variables that may
be set on a repo. Typically, it is best practice to set them at
repo init time.

> Right, for now legacy:13 is still the default, but as mentioned in the
> pending release notes (note/main.md):
>
> * The deduplication granularity can now be changed by a new
> `bup.split.files` configuration option which defaults to `legacy:13`
> (the current behavior), but should probably be set to a higher value
> like `legacy:16` in new repositories (say via `git-config --file
> REPO/config bup.split.files legacy:16`). The default for new
> repositories will eventually be raised. See `bup-config`(5) for
> additional information.

I was reading the pending docs and not the release notes. I had
multiple problems: 1) not knowing bup config syntax for multilevel
variables (my error) and 2) being unclear that that "16" was not
acceptable.

I would suggest changing

legacy refers to the current split method, which has an uninten‐
tional quirk where it “skips a bit”.

to something like

legacy refers to the current split method, which has a bug where
asking for 13 bits resulting in splitting when only 12 bits are all 1.
bup chooses to be consistent; if you want idx files to take up 8x less
space on disk and in memory, pick 16, and if you want 32x less, pick
18.

if that's what it is. "skips a bit" is too hard to understand.

and maybe

Thus, to ask for hash splitting to 16 bits (using the historical
code), use `git-config --file REPO/config bup.split.files legacy:16`.

> bup.split.files is just a git config values, so you set it with
> git-config, and it's documented in bup-config(5) (guessing you found
> that). Though we should mention git-config(1) and bup-config(5) in
> bup-init(1).

I favor mentioninin bup-config loudly as I suggest above and letting
that refer to git-config(1).

> For example:
>
> git --git-dir="$BUP_DIR" config bup.split.files legacy:16
> git --git-dir="$BUP_DIR" config bup.split.trees true

would be great to have those in bup-config(5).

>> a) Why does bup init, when using -r, touch *at all* or require
>> anything about a local repo? If I'm initializing a remote, shouldn't
>> it just do that?
>
> If I recall correctly, this was added to just clearly document
> long-standing behavior (related to the original/previous emphasis on
> ~/.bup). I suspect that's not what we'd choose now.
>
>> willing to initialize ~/.bup if path is not given, but not path,
>> seems like too much magic. I vote for no implicit initialization.
>
> I tend to agree, were we starting from scratch, but that would be a
> notable break in backward compatibility.

I suspect almost zero are relying on implicit init in scripts, and those
could adjust easily. I realize it's a tough call; I generally lean to
clean up semantics if the total pain imposed (amount * people) is
smaller than the cognitive load on everyone else.

Are you really saying that you can do

bup save -r host:

when there is no ~/.bup in host, and it will do an initialization first?
But that if you do

bup save -r host:.bup

then it will error out?

>> d) the default about SSH seems outdated; I suggest deleting the
>> sentence as the "See bup(1) REMOTE OPERATIONS" is sufficient and
>> doesn't risk outdated/conflicting information. (Plus SSH in caps is
>> unusual.)
>
> I'm not sure I follow. I think this is still just referring to ~/.bup,
> which would be automatically created in the past, though now that I
> think about it, we actually have changed some of that, so I should
> double-check the current situation...

I meant to suggest:

Delete "By default the connection to the remote server is made with
SSH." so that one leaves the description to bup(1).

because -r is more complicated, and ssh is so normal that not-ssh is
odd.

[Rob Browning]

Greg Troxel <g...@lexort.com> writes:

> I guess I should have said more coherently: if we mean, as I think we
> do, that it is safe and reasonable to call bup init on an already
> initialized repository, then it would be good to simply say that. I
> would expect many readers to think it scary to call init on a place with
> existing data.

I understood, and I just meant that until this recent work, I hadn't
worked more carefully with init, and didn't know what we had intended,
or was feasible. I know git itself handles reinitialization, but wasn't
completely familiar with the promises there either. And with -r and
~/.bup we have additional existing complexity/behavior to consider.

So I've been conservative about making commitments. And while I think it
should be safe to reinitialize, we've recently done related work there,
and I think we probably will formalize that, I wasn't ready to promise
it yet.

It's also the case that part of what we would/will be promising is
"whatever git-init does".

> It is not fair, but I was seeing this as a bit of a regression, because
> I'm moving from hand-patched to 16 to trying to use the new mechanism.

Ahh, right. Suppose for now you might want a bup init wrapper or
similar.

> Now, I'm not finding the bup-config xref.
>
> Perhaps add
>
> See bup-config(5) for a description of configuration variables that may
> be set on a repo. Typically, it is best practice to set them at
> repo init time.

Agreed, I'd started adding something like that, but not the note about
setting them during initialization. Good idea, and I'll probably
augment it a bit further.

> I was reading the pending docs and not the release notes. I had
> multiple problems: 1) not knowing bup config syntax for multilevel
> variables (my error) and 2) being unclear that that "16" was not
> acceptable.

I think I'll contemplate adding an example or two somewhere.

> if that's what it is. "skips a bit" is too hard to understand.

I think that legacy:16 still works with 16 bits, it just ignores a bit
in the broader computation. This is mentioned in DESIGN (thanks to
Johannes):

What we do instead is we extend the hashsplit algorithm a little
further using what we call "fanout." Instead of checking just the last
13 bits of the checksum, we use additional checksum bits to produce
additional splits. Note that (most likely due to an implementation
bug), the next higher bit after the 13 bits (marked 'x'):

...... '..x1'1111'1111'1111

is actually ignored (so the fanout starts just to the left of the
x). Now, let's say we use a 4-bit fanout. That means we'll break a
series of chunks into its own tree object whenever the next 4 bits of
the rolling checksum are 1, in addition to the 13 lowest ones. Since
the 13 lowest bits already have to be 1, the boundary of a group of
chunks is necessarily also always the boundary of a particular chunk.

> I suspect almost zero are relying on implicit init in scripts, and those
> could adjust easily. I realize it's a tough call; I generally lean to
> clean up semantics if the total pain imposed (amount * people) is
> smaller than the cognitive load on everyone else.
>
> Are you really saying that you can do
>
> bup save -r host:
>
> when there is no ~/.bup in host, and it will do an initialization first?

I'm not positive, but yes, I think so, and likely in any number of other
cases. I believe we used to implicitly try to create ~/.bup in one of
the main lower-level "find the repo" functions.

But I hesitate(d) because we've also "dialed down" the ~/.bup automagic
in some places, so I was wondering if that might have invalidated the
bup-init(1) claim, and sure enough (0.33.7):

$ ls -l ~/.bup
ls: cannot access '/home/rlb/.bup': No such file or directory
$ bup -d ~/tmp/bup-test-repo save -r localhost: -n lib lib
error: repository b'/home/rlb/.bup/' does not exist (see "bup help init")
error: server tunnel returned exit code 15

I was also able to provoke one of the errors you saw when I had the
remote bup set to main in the PATH:

$ bup -d ~/tmp/test-repo save -r localhost: -n lib lib
error: /home/rlb/.bup/ is missing (see "bup help init")
Exception ignored in: <function LocalRepo.__del__ at 0x7f3eb7a89da0>
Traceback (most recent call last):
File "/home/rlb/src/bup/main/lib/bup/repo/local.py", line 67, in __del__
def __del__(self): assert self.closed
AttributeError: 'ServerRepo' object has no attribute 'closed'

If that's repeatable, I can investigate, but it might be "expected"
right now cross-version. As mentioned, it should be harmless, if
confusing, and I'll have to think about what we want to do. Those
errors are intended to help make sure we always explicitly manage
relevant resources, and have helped notably improve that in main.

> I meant to suggest:
>
> Delete "By default the connection to the remote server is made with
> SSH." so that one leaves the description to bup(1).
>
> because -r is more complicated, and ssh is so normal that not-ssh is
> odd.

Ahh, OK.

[Greg Troxel]

Rob Browning <r...@defaultvalue.org> writes:

> Greg Troxel <g...@lexort.com> writes:
>
>> I guess I should have said more coherently: if we mean, as I think we
>> do, that it is safe and reasonable to call bup init on an already
>> initialized repository, then it would be good to simply say that. I
>> would expect many readers to think it scary to call init on a place with
>> existing data.
>
> I understood, and I just meant that until this recent work, I hadn't
> worked more carefully with init, and didn't know what we had intended,
> or was feasible. I know git itself handles reinitialization, but wasn't
> completely familiar with the promises there either. And with -r and
> ~/.bup we have additional existing complexity/behavior to consider.
>
> So I've been conservative about making commitments. And while I think it
> should be safe to reinitialize, we've recently done related work there,
> and I think we probably will formalize that, I wasn't ready to promise
> it yet.
>
> It's also the case that part of what we would/will be promising is
> "whatever git-init does".

I guess we need to either say

bup init on an existing repo is safe

this week bup init on existing repos is ok, but you never know about
tomorrow -- do don't do it.

bup init on an existing repo is not safe and should never happen, and
the code checks for it and errors out

and if we don't say it's ok, then we should have

bup upgrade-repo

which is defined to be safe and which does things like setting pack
versions and repo.id -- and doesn't call git init.

>> It is not fair, but I was seeing this as a bit of a regression, because
>> I'm moving from hand-patched to 16 to trying to use the new mechanism.
>
> Ahh, right. Suppose for now you might want a bup init wrapper or
> similar.

Yes, I think I just want a script that calls bup init and then git
config for bup.fils.split.

> I think I'll contemplate adding an example or two somewhere.

A single example to set legacy:16 would do wonders.

>> if that's what it is. "skips a bit" is too hard to understand.
>
> I think that legacy:16 still works with 16 bits, it just ignores a bit
> in the broader computation. This is mentioned in DESIGN (thanks to
> Johannes):
>
> What we do instead is we extend the hashsplit algorithm a little
> further using what we call "fanout." Instead of checking just the last
> 13 bits of the checksum, we use additional checksum bits to produce
> additional splits. Note that (most likely due to an implementation
> bug), the next higher bit after the 13 bits (marked 'x'):
>
> ...... '..x1'1111'1111'1111
>
> is actually ignored (so the fanout starts just to the left of the
> x). Now, let's say we use a 4-bit fanout. That means we'll break a
> series of chunks into its own tree object whenever the next 4 bits of
> the rolling checksum are 1, in addition to the 13 lowest ones. Since
> the 13 lowest bits already have to be 1, the boundary of a group of
> chunks is necessarily also always the boundary of a particular chunk.

I'm assuming that the fanout is fixed. Otherwise the trees won't line
up for other writers.

Thus for blobs hashsplit legacy:13 really is 13. It's just that instead
of starting a new tree when hashpslit 17 would start a blob, it starts
it under slightly different conditions: hashsplit 18 would start, or
hashsplit 18 if the 14th bit were inverted.

It's hard to do the math confidently in my head, but I think this
results in the same average tree size as if the bug were not there.

So I would say:

The token legacy:N specifies that we match historical code which has a
harmless bug in grouping blobs into trees. While the overall sizes
are right, an incorrect higher-order bit is used, and the correct
algorithm would group differently. As the whole point is to
deduplicate the same way, one should use legacy:N, so strongly that no
other algorithm has yet been implemented in bup.

if that's correct. "skips a bit" implies that hashsplit 13 is behaving
like hashsplit 12 should behave.

My take is:

- drop docs that imply implicit bup init
- remove any implicit init calls

- change the __del__ member to do nothing if there is no state
recorded that it was opened, assuming it's a class destructor
(the first error line is great)

[Rob Browning]

Greg Troxel <g...@lexort.com> writes:

> if that's correct. "skips a bit" implies that hashsplit 13 is behaving
> like hashsplit 12 should behave.

How about just:

`legacy` refers to the current split method, which has an
unintentional, but harmless quirk. See DESIGN in the source tree
for further details.

> - change the __del__ member to do nothing if there is no state
> recorded that it was opened

That's what it's supposed to do. It's just an "assert
wasactuallyclosed", and it's being called from __del__ (i.e. during gc,
so we must have forgotten to explicitly clean it up).

It's "harmless" because python doesn't treat exceptions during __del__
as errors, which is just what we want in this case.

(And of course, now I can't reproduce the __del__ error either.)

[Rob Browning]

Greg Troxel <g...@lexort.com> writes:

> My take is:
>
> - drop docs that imply implicit bup init
> - remove any implicit init calls

OK, so as I thought, we did remove the implicit ~/.bup initialization
for local init calls in

621fbd9f86da68e7bc6974fe59cf9e32c29cdd3c
git.py: don't automatically initialize ~/.bup if it doesn't exist.

But we still do what init claims for --remote (with at least 0.33.7):

$ rm -rf ~/.bup
$ bup init -r localhost:
Initialized empty Git repository in /home/rlb/.bup/

However, if we use some other user, it crashes:

$ rm -rf ~/.bup
$ bup init -r testbup@localhost:
Initialized empty Git repository in /home/rlb/.bup/
error: repository b'/home/testbup/.bup/' does not exist (see "bup help init")

Traceback (most recent call last):

File "/usr/lib/bup/bup/client.py", line 183, in check_ok
...

...presumably because the explicitly created local ~/.bup no longer
helps with the second client-related dependency in the other HOME, and
we no longer implicitly create ~/.bup.

With an explicit target:

$ rm -rf ~/.bup
$ bup init -r localhost:/home/rlb/tmp/bup-tmp-repo
Initialized empty Git repository in /home/rlb/.bup/
Initialized empty Git repository in /home/rlb/tmp/bup-tmp-repo/

Bup that also crashes when it's some other user. I'll plan to
investigate.

[Greg Troxel]

Rob Browning <r...@defaultvalue.org> writes:

> Greg Troxel <g...@lexort.com> writes:
>
>> if that's correct. "skips a bit" implies that hashsplit 13 is behaving
>> like hashsplit 12 should behave.
>
> How about just:
>
> `legacy` refers to the current split method, which has an
> unintentional, but harmless quirk. See DESIGN in the source tree
> for further details.

That's fine. Without saying it's still current, one would ask "What's
the replacement? Obviously it's better and I should be using it. How
do I do that?" and that minimally heads that off.

>> - change the __del__ member to do nothing if there is no state
>> recorded that it was opened
>
> That's what it's supposed to do. It's just an "assert
> wasactuallyclosed", and it's being called from __del__ (i.e. during gc,
> so we must have forgotten to explicitly clean it up).

Is the create method involved in handling closed?

Is there some other constructor?

I find closed to have the wrong sense; what's natural to me is to have a
variable that is True if resources are allocated that need to be
freed/closed. But that's not wrong, just confusing to me.

[Greg Troxel]

I would think the local and remote are totally unrelated, so this is puzzling.

With things to set in a repo (split variables), having a way to create a
repo that does not allow one to configure it seems problematic.

Does anybody really use bup init -r? I'd be tempted to withdraw it as
no longer working with the new world of configurable repos.

[Rob Browning]

Greg Troxel <g...@lexort.com> writes:

> Is the create method involved in handling closed?

Our handling of these markers has changed over time, but with LocalRepo
as an example, __init__ is involved, and sets self.closed = True as the
very first step, so that there's only a narrow window after __new__()
where it might be left unset, causing close() to crash. We could also
eliminate that case via an additional attr existence check in close().

And then __init__ only sets closed to False once the instance is "ready"
for close to take over cleanup. All incremental cleanup in init until
that point should be handled carefully (i.e. usually by context
management). LocalRepo doesn't have much of that intermediate state,
but other classes do.

Though the handling in other classes varies right now. I think some
invert the sequence, setting closed to False first, but managing the
resources in a way that's still compatible with their close().

Of late, I've been tending to shift much of the work toward just having
an ExitStack handle it, one that's built up during init, and then
"handed over" to close via (generally) pop_all().

[Rob Browning]

Greg Troxel <g...@lexort.com> writes:

> I would think the local and remote are totally unrelated, so this is puzzling.

I think I understand what's happening now. There are some changes in
main that need to be adjusted with respect to init, and as a practical
matter, "init -r host:" with no path hasn't worked since at least 0.33.7
(it crashes in 0.33.x and in main).

I'm not certain, but I think I'm going to fix "init -r host:" to default
to remote ~/.bup, because even if I might not choose that now, the other
-r arguments do (and document it, e.g. save -r).

But we'll still no longer support implict creation of ~/.bup (i.e. you
have to "init -r host:" before your first "save -r host:".

[Greg Troxel]

Rob Browning <r...@defaultvalue.org> writes:

> But we'll still no longer support implict creation of ~/.bup (i.e. you
> have to "init -r host:" before your first "save -r host:".

in the brave new world of git-config'ing a new bupdir, how do you do
that with init -r?

[Rob Browning]

Greg Troxel <g...@lexort.com> writes:

> in the brave new world of git-config'ing a new bupdir, how do you do
> that with init -r?

You'd still have to ssh to the remote host for that.

For now, we'd just be fixing "init -r host": (which I suspect used to
work, but haven't verified; the older versions won't build anymore with
my current gcc), and making it consistent with the other --remote
arguments.

Assuming we get back to Johannes' remote repositories in a bit, we'll
have even more questions to consider with respect to "configuration".

[Greg Troxel]

Rob Browning <r...@defaultvalue.org> writes:

> Greg Troxel <g...@lexort.com> writes:
>
>> in the brave new world of git-config'ing a new bupdir, how do you do
>> that with init -r?
>
> You'd still have to ssh to the remote host for that.
>
> For now, we'd just be fixing "init -r host": (which I suspect used to
> work, but haven't verified; the older versions won't build anymore with
> my current gcc), and making it consistent with the other --remote
> arguments.

I can understand that; I question the wisdom of having init -r at all,
but it really doesn't matter.

> Assuming we get back to Johannes' remote repositories in a bit, we'll
> have even more questions to consider with respect to "configuration".

Agreed, and I think the first step is to have a "bup config" because
while "bup uses git datastructures" it is a bug to break the abstraction
boundary to the user.

[Rob Browning]

Have init --remote without a path default to ~/.bup as the other
--remote arguments (e.g. save) do. Previously (e.g. in 0.33.x) this
would just crash. Improve the related documentation and augment
test/ext/bup-init.

Thanks to Greg Troxel for suggesting improvements to the
documentation.

Signed-off-by: Rob Browning <r...@defaultvalue.org>
Tested-by: Rob Browning <r...@defaultvalue.org>
---
Documentation/bup-init.1.md | 14 +++---
Documentation/bup.1.md | 6 +--
lib/bup/client.py | 14 +++---
note/main.md | 11 +++++
test/ext/test-init | 92 ++++++++++++++++++++++++++++++-------
5 files changed, 103 insertions(+), 34 deletions(-)

diff --git a/Documentation/bup-init.1.md b/Documentation/bup-init.1.md
index 2c33631e..25563c6e 100644
--- a/Documentation/bup-init.1.md
+++ b/Documentation/bup-init.1.md
@@ -13,18 +13,16 @@ bup init [-r *host*:*path*] [*directory*]
# DESCRIPTION

`bup init` initializes a repository. The location will be the
-`*directory*` if provided, the directory specifed by any global `-d`
-argument (see `bup`(1)), the value of `BUP_DIR` in the environment if
-set, or `~/.bup`.
+`*directory*` or `--remote` if provided, the directory specifed by any
+global `-d` argument (see `bup`(1)), the value of `BUP_DIR` in the
+environment if set, or `~/.bup`.

# OPTIONS

-r, \--remote=[*user*@]*host*:[*path*], \--remote=URL
-: Initialize not only the local repository, but also the specified
- remote repository. This is not necessary if you intend to use the
- default location on the server (ie. with no *path*). By default
- the connection to the remote server is made with SSH. See bup(1)
- REMOTE OPTIONS for further information.
+: Initialize the specified *path* on the given *host*. Incompatible
+ with *directory*. See bup(1) REMOTE OPTIONS
+ for further information.

# EXAMPLES
bup init ~/archive
diff --git a/Documentation/bup.1.md b/Documentation/bup.1.md
index a6e22d4d..6bdc33f1 100644
--- a/Documentation/bup.1.md
+++ b/Documentation/bup.1.md
@@ -162,9 +162,9 @@ remote path as either a URL (see `REPOSITORY URLS` below) or a
`[*user*@]*host*:[*path*]`.

For either format, when there is no path, the default path on the
-server will be used, and SSH settings for the connection can be
-provided by a custom host to your `~/.ssh/config` file
-(`ssh_config(5)`).
+server will be used (`BUP_DIR` if set in the remote environment or
+`~/.bup`), and SSH settings for the connection can be provided by a
+custom host in your `~/.ssh/config` file (`ssh_config(5)`).

The argument is treated as a URL if it begins with a syntactically
valid URL scheme prefix that contains an "authority" (meaning that it
diff --git a/lib/bup/client.py b/lib/bup/client.py
index 1f1ea3dc..562c3663 100644
--- a/lib/bup/client.py
+++ b/lib/bup/client.py
@@ -326,14 +326,16 @@ class Client:
ctx.enter_context(self._transport)
self.conn = self._transport.conn
self._available_commands = self._get_available_commands()
+ mangled_path = b''
if self.path:
mangled_path = re.sub(br'[\r\n]', b' ', self.path)
- if create:
- self._require_command(b'init-dir')
- self.conn.write(b'init-dir %s\n' % mangled_path)
- else:
- self._require_command(b'set-dir')
- self.conn.write(b'set-dir %s\n' % mangled_path)
+ if create:
+ self._require_command(b'init-dir')
+ self.conn.write(b'init-dir %s\n' % mangled_path)
+ self.check_ok()
+ elif self.path:
+ self._require_command(b'set-dir')
+ self.conn.write(b'set-dir %s\n' % mangled_path)
self.check_ok()
if url.scheme == b'bup-rev':
legacy_id = _legacy_cache_id_for_remote(url.host, True)
diff --git a/note/main.md b/note/main.md
index 54b8a692..1e20dc7c 100644
--- a/note/main.md
+++ b/note/main.md
@@ -18,6 +18,12 @@ May require attention
commits if the `DEST` was not itself a commit (the parent would be
whatever `DEST` initially pointed to).

+* `bup init -r` (`--remote`) now only initializes the remote
+ repository, not the local and remote repositories, so `bup init -r
+ host:remote` and `bup -d local init -r host:remote` will only
+ initialze the `remote` repository, not `~/.bup` and `./local`
+ respectively.
+
* Following POSIX `ls`, `bup ls -l` no longer dereferences symlinks,
for example for `bup ls -l save/latest` (`bup ls -l save/latest/`
still does).
@@ -268,6 +274,11 @@ Bugs
dates). Previously it would fail with a message like "error: cannot
access SAVE in SAVE".

+* `bup init -r host:` (without a path) now initializes the default
+ remote repository (remote `BUP_DIR` or `~/.bup`). Previously it
+ would crash. This matches the behavior of `-r` for other commands
+ like `bup save -r`.
+
* When run on an existing repository, `bup init` will no longer change
existing `core.logAllRefUpdates` settings.

diff --git a/test/ext/test-init b/test/ext/test-init
index 51f61770..e0df05b9 100755
--- a/test/ext/test-init
+++ b/test/ext/test-init
@@ -1,6 +1,7 @@
#!/usr/bin/env bash
. ./wvtest-bup.sh || exit $?
. ./dev/lib.sh || exit $?
+. ./test/lib/btl.sh

set -o pipefail

@@ -9,6 +10,15 @@ tmpdir="$(WVPASS wvmktempdir)" || exit $?

bup() { "$top/bup" "$@"; }

+validate_repo()
+{
+ local dir="$1"
+ WVPASS test -d "$dir"
+ WVPASS test -f "$dir/HEAD"
+ WVPASS test -f "$dir/config"
+ WVPASS test -d "$dir/objects/pack"
+}
+
WVPASS cd "$tmpdir"


@@ -17,46 +27,94 @@ WVPASS mkdir foo
WVEXPRC 2 bup -d nope save -t foo
WVPASS rmdir foo

+WVSTART 'only one positional argument allowed'
+WVFAIL err-to log bup init x y
+WVPASS test ! -e x
+WVPASS test ! -e y
+WVPASS grep 'only the directory positional argument is allowed' log
+
+WVSTART 'local and remote disallowed'
+WVFAIL err-to log bup init -r -:remote local
+WVPASS test ! -e remote
+WVPASS test ! -e local
+WVPASS grep 'cannot initialize both local and remote repo' log
+
+WVSTART 'initialization failure'
+WVEXPRC "$bup_exit_failure" err-to log bup init /dev/null
+WVPASS grep "could not init repository" log
+
WVSTART '-d repo argument'
WVPASS test ! -e repo
WVPASS bup -d repo init
-WVPASS test -d repo/refs/heads
-WVPASS test -d repo/objects/pack
+validate_repo repo
WVPASS rm -rf repo

WVSTART 'positional repo argument'
WVPASS test ! -e repo
WVPASS bup init repo
-WVPASS test -d repo/refs/heads
-WVPASS test -d repo/objects/pack
+validate_repo repo
WVPASS rm -rf repo

-WVSTART 'positional repo argument precdence over -d'
+WVSTART 'positional repo argument precedence over -d'
WVPASS test ! -e repod
WVPASS test ! -e repo
WVPASS bup -d repod init repo
WVPASS test ! -e repod
-WVPASS test -d repo/refs/heads
-WVPASS test -d repo/objects/pack
+validate_repo repo
WVPASS rm -rf repo

-WVSTART 'positional repo argument precdence over BUP_DIR'
+WVSTART 'positional repo argument precedence over BUP_DIR'
WVPASS test ! -e repod
WVPASS test ! -e repo
-export BUP_DIR=repod
-WVPASS bup init repo
-unset BUP_DIR
-WVPASS test ! -e repod
-WVPASS test -d repo/refs/heads
-WVPASS test -d repo/objects/pack
+(export BUP_DIR=repod
+ WVPASS bup init repo
+ WVPASS test ! -e repod
+ validate_repo repo)
WVPASS rm -rf repo

-WVSTART '--remote'
+WVSTART '--remote repo argument precedence over BUP_DIR'
+WVPASS rm -rf repo
+WVPASS test ! -e repo
+WVPASS test ! -e remote
+(export BUP_DIR=repo
+ WVPASS bup init --remote -:remote
+ validate_repo remote
+ WVPASS test ! -e repo)
WVPASS rm -rf repo
-WVPASS bup init --remote -:repo

-WVEXPRC "$bup_exit_failure" bup init /dev/null
+WVSTART 'no --remote path'
+WVPASS rm -rf home
+WVPASS mkdir home
+(unset BUP_DIR
+ export HOME="$(pwd)/home" # works because - isn't ssh
+ WVPASS bup init --remote -:
+ validate_repo home/.bup)
+WVPASS rm -rf home
+
+if test -z "$BUP_TEST_OTHER_BUP"; then
+ WVMSG 'Skipping --remote against other bup (no BUP_TEST_OTHER_BUP)'
+else
+ other_bup="$(printf "%q" "$BUP_TEST_OTHER_BUP")"
+ WVSTART "--remote -: against $other_bup"
+ WVPASS rm -rf home
+ WVPASS mkdir home
+ (unset BUP_DIR
+ export HOME="$(pwd)/home" # works because - isn't ssh
+ export BUP_TEST_SSH_BUP_PATH="$BUP_TEST_OTHER_BUP"
+ WVPASS bup init --remote -:
+ validate_repo home/.bup)
+ WVPASS rm -rf home

+ WVSTART "--remote -:path against $other_bup"
+ WVPASS rm -rf repo
+ WVPASS test ! -e repo
+ WVPASS test ! -e "$HOME/.bup"
+ (export BUP_TEST_SSH_BUP_PATH="$BUP_TEST_OTHER_BUP"
+ WVPASS bup init --remote -:repo
+ validate_repo repo
+ WVPASS test ! -e "$HOME/.bup")
+ WVPASS rm -rf repo
+fi

WVPASS cd "$top"
WVPASS rm -rf "$tmpdir"
--
2.47.3

[Rob Browning]

Signed-off-by: Rob Browning <r...@defaultvalue.org>
---

Pushed to main.

Documentation/bup-config.5.md | 35 ++++++++++++++++++++++++++---------
Documentation/bup-init.1.md | 6 +++++-
2 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/Documentation/bup-config.5.md b/Documentation/bup-config.5.md
index 426299c5..c489de85 100644
--- a/Documentation/bup-config.5.md
+++ b/Documentation/bup-config.5.md
@@ -9,7 +9,9 @@ bup-config - bup configuration options
# DESCRIPTION

The following options may be set in the relevant `git` config
-(`git-config(1)`).
+(`git-config(1)`). For example:
+
+ git --git-dir="$BUP_DIR" config bup.split.trees true

# OPTIONS

@@ -51,12 +53,15 @@ bup.server.deduplicate-writes (default `true`)

bup.split.files (default `legacy:13`)
: Method used to split data for deduplication, for example by `bup
- save` or `bup split`. This determines the "granularity" of the
- deduplication, with larger values producing, on average, larger
- chunks. The value must be a string like `legacy:N` where the
- integer `N` must be greater than 12 and less than 22. The default
- of 13 provides backward compatibility, but it is recommended to
- increase this, say to 16, for all but very small repos.
+ save` or `bup split`. Should not normally be changed after adding
+ any data to the repository (see below).
+
+ This determines the "granularity" of the deduplication, with
+ larger values producing, on average, larger chunks. The value must
+ be a string like `legacy:N` where the integer `N` must be greater
+ than 12 and less than 22. The default of 13 provides backward
+ compatibility, but it is recommended to increase this for larger
+ repositories.

`N` specifies the number of fixed bits in the hash-split algorithm
that when all set to one produce a chunk boundary, and thus it
@@ -65,13 +70,25 @@ bup.split.files (default `legacy:13`)
(fewer bits means better deduplication) as compared to the amount
of metadata to keep on disk and the RAM usage during repo
operations (more bits means fewer objects, means less metadata
- space and RAM use). The expected average block size is 2^bits (1
+ space and RAM use). The expected average blob size is 2^bits (1
<< bits). A sufficiently small change in a file would cause that
much new data to be saved (plus tree metadata). The maximum blob
size is four times that.

+ The historical default of `legacy:13` is probably small for many
+ current repositories. As mentioned above, setting it higher
+ should decrease the RAM required for many operations by roughly a
+ factor of two per increment while also increasing the repository's
+ size by some amount (because it exposes less potential
+ deduplication), but the effect depends on the data stored in the
+ repository (file sizes, deduplication rates, etc.). If you have
+ the space and time, you can always test different values for your
+ data by comparing `bup get --rewrite`s to new repositories with
+ different settings.
+

`legacy` refers to the current split method, which has an

- unintentional quirk where it "skips a bit".
+ unintentional, but harmless quirk. See DESIGN in the source tree
+ for further details.

*NOTE:* Changing this value in an existing repository will
duplicate data because it causes the split boundaries to change,
diff --git a/Documentation/bup-init.1.md b/Documentation/bup-init.1.md
index 25563c6e..ffd051e2 100644
--- a/Documentation/bup-init.1.md
+++ b/Documentation/bup-init.1.md
@@ -17,6 +17,10 @@ bup init [-r *host*:*path*] [*directory*]

global `-d` argument (see `bup`(1)), the value of `BUP_DIR` in the

environment if set, or `~/.bup`.

+See `bup-config(5)` for configuration options that you may want to
+set. Some, like `bup.split.files` and `bup.split.trees` should
+normally be set (when desired) before adding data to the repository.
+

# OPTIONS

-r, \--remote=[*user*@]*host*:[*path*], \--remote=URL

@@ -29,7 +33,7 @@ environment if set, or `~/.bup`.

# SEE ALSO

-`bup-fsck`(1), `ssh_config`(5)
+`bup-config(5)`, `ssh_config`(5)

# BUP

--
2.47.3