[PATCH 05/17] _tty_width: return 70 if get_terminal_size returns zero

[Rob Browning]

Apparently some versions of python can return zero from
_tty_width. Just return the same default (70) for that case that we do
for OSError (not a terminal). The zero crashed the textwrap call.

cf. https://github.com/python/cpython/issues/86340

Signed-off-by: Rob Browning <r...@defaultvalue.org>
Tested-by: Rob Browning <r...@defaultvalue.org>
---
lib/bup/options.py | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/bup/options.py b/lib/bup/options.py
index 890c3b8b..1e357513 100644
--- a/lib/bup/options.py
+++ b/lib/bup/options.py
@@ -128,9 +128,11 @@ def _intify(v):
def _tty_width():
try:
size = os.get_terminal_size(sys.stderr.fileno())
+ # May return 0 for some python versions:
+ # https://github.com/python/cpython/issues/86340
+ return size.columns or 70
except OSError:
return 70
- return size.columns


class Options:
--
2.47.3

[Rob Browning]

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

(cherry picked from commit 24efccc9ea8051b3a99d70ebac26e0365c610f0d)
---
test/ext/test-misc | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/test/ext/test-misc b/test/ext/test-misc
index 92094b43..60427429 100755
--- a/test/ext/test-misc
+++ b/test/ext/test-misc
@@ -129,19 +129,18 @@ b"


WVSTART "import-rsnapshot"
-D=rsnapshot.tmp
-export BUP_DIR="$tmpdir/$D/.bup"
-WVPASS force-delete $D
-WVPASS mkdir $D
+export BUP_DIR="$tmpdir/rsnapshot/.bup"
+WVPASS rm -rf rsnapshot
+WVPASS mkdir rsnapshot
WVPASS bup init
-WVPASS mkdir -p $D/hourly.0/buptest/a
-WVPASS touch $D/hourly.0/buptest/a/b
-WVPASS mkdir -p $D/hourly.0/buptest/c/d
-WVPASS touch $D/hourly.0/buptest/c/d/e
-WVPASS true
-WVPASS bup import-rsnapshot $D/
+WVPASS mkdir -p rsnapshot/hourly.0/buptest/a
+WVPASS touch rsnapshot/hourly.0/buptest/a/b
+WVPASS mkdir -p rsnapshot/hourly.0/buptest/c/d
+WVPASS touch rsnapshot/hourly.0/buptest/c/d/e
+WVPASS bup import-rsnapshot rsnapshot/
WVPASSEQ "$(bup ls -F buptest/latest/)" "a/
c/"
+WVPASS rm -rf rsnapshot


WVSTART features
--
2.47.3

[Rob Browning]

Signed-off-by: Rob Browning <r...@defaultvalue.org>
---
lib/bup/cmd/get.py | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/bup/cmd/get.py b/lib/bup/cmd/get.py
index 5cdfaa45..1a16dfd5 100644
--- a/lib/bup/cmd/get.py
+++ b/lib/bup/cmd/get.py
@@ -224,7 +224,8 @@ def get_random_item(name, hash, src_repo, dest_repo, opt):
# attempts to write some other oid that the server already
# has. Without this check, we can provoke a duplicate index
# suggestion which then causes client.sync_index() to throw.
- if item.type != b'blob' and dest_repo.exists(item.oid):
+ if not isinstance(dest_repo, LocalRepo) and item.type != b'blob' \
+ and dest_repo.exists(item.oid):
continue
dest_repo.just_write(item.oid, item.type, item.data)

--
2.47.3

[Rob Browning]

Suspect this is no longer needed and it was causing failures in some
environments where modprobe wasn't available:
https://bugs.debian.org/1111625

Thanks to Jochen Sprickerhof for the report.

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

---
test/int/test_metadata.py | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/test/int/test_metadata.py b/test/int/test_metadata.py
index e6147a2c..3f5b5c03 100644
--- a/test/int/test_metadata.py
+++ b/test/int/test_metadata.py
@@ -37,10 +37,8 @@ def ex(*cmd):


def setup_testfs():
+ # Try to set up testfs with user_xattr, etc.
assert(sys.platform.startswith('linux'))
- # Set up testfs with user_xattr, etc.
- if subprocess.call([b'modprobe', b'loop']) != 0:
- return False
subprocess.call([b'umount', b'testfs'])
ex(b'dd', b'if=/dev/zero', b'of=testfs.img', b'bs=1M', b'count=32')
ex(b'mke2fs', b'-F', b'-j', b'-m', b'0', b'testfs.img')
@@ -272,7 +270,7 @@ if xattr:
pytest.skip('skipping test -- not superuser')
return
if not setup_testfs():
- pytest.skip('unable to load loop module; skipping dependent tests')
+ pytest.skip('unable to set up test fs; skipping dependent tests')
return
for f in glob.glob(b'testfs/*'):
ex(b'rm', b'-rf', f)
--
2.47.3

[Rob Browning]

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

lib/bup/metadata.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/bup/metadata.py b/lib/bup/metadata.py
index c8ebaa3b..7a275657 100644
--- a/lib/bup/metadata.py
+++ b/lib/bup/metadata.py
@@ -1176,7 +1176,7 @@ def display_archive(file, out):
for meta in _ArchiveIterator(file):
if not meta.path:
log('bup: no metadata path, but asked to only display path'
- ' (increase verbosity?)')
+ ' (increase verbosity?)\n')
sys.exit(EXIT_FAILURE)
out.write(meta.path)
out.write(b'\n')
--
2.47.3

[Rob Browning]

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

lib/bup/gc.py | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/lib/bup/gc.py b/lib/bup/gc.py
index a86af9c3..e4684b94 100644
--- a/lib/bup/gc.py
+++ b/lib/bup/gc.py
@@ -3,6 +3,7 @@ from binascii import hexlify, unhexlify
from contextlib import ExitStack
#from itertools import chain
from os.path import basename
+from stat import S_ISDIR
import glob, os, re, subprocess, sys, tempfile

from bup import bloom, git, midx
@@ -72,8 +73,12 @@ def report_missing(ref_name, item_path):
i = len(item_path) - 1
while i >= 0 and item_path[i].type != b'commit':
i -= 1
- path = b'/'.join(item.name for item in item_path[i:])
- note_error(f'missing {ref}:{path}/\n')
+ path = path_msg(b'/'.join(x.name for x in item_path[i:]))
+ item = item_path[-1]
+ if S_ISDIR(item.mode):
+ note_error(f'missing {item.oid.hex()} {ref}:{path}/\n')
+ else:
+ note_error(f'missing {item.oid.hex()} {ref}:{path}\n')


def find_live_objects(existing_count, cat_pipe, idx_list, refs=None,
--
2.47.3

[Rob Browning]

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

dev/sparse-size | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/dev/sparse-size b/dev/sparse-size
index ebb9c409..13aae7f5 100755
--- a/dev/sparse-size
+++ b/dev/sparse-size
@@ -73,11 +73,12 @@ with open(opt.path, 'rb') as f:
try:
new_off = lseek(fd, off, SEEK_DATA)
except OSError as ex:
- if ex.errno == ENXIO:
- if opt.verbose:
- log(f'hole: {end - off} @ {off}\n')
- sparse += end - off
- break
+ if ex.errno != ENXIO:
+ raise ex
+ if opt.verbose:
+ log(f'hole: {end - off} @ {off}\n')
+ sparse += end - off
+ break
if opt.verbose:
log(f'hole: {new_off - off} @ {off}\n')
sparse += new_off - off
--
2.47.3

[Rob Browning]

A collection of assorted pending improvements and bug fixes. Proposed
for main.

--
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

[Rob Browning]

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

test/ext/test_get.py | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/test/ext/test_get.py b/test/ext/test_get.py
index aec9faf8..7b29e99b 100644
--- a/test/ext/test_get.py
+++ b/test/ext/test_get.py
@@ -121,7 +121,7 @@ def validate_commit(src_id, dest_id):
src_cat = exr.out
exr = verify_rcz((b'git', b'--git-dir', b'get-dest', b'cat-file', b'commit', dest_id))
# Check parent connectivity, etc.
- ex((b'git', b'--git-dir', b'get-dest', b'log', b'-n2', dest_id),
+ ex((b'git', b'-P', b'--git-dir', b'get-dest', b'log', b'-n2', dest_id),
stdin=DEVNULL)

if exr.rc != 0: return False
@@ -153,7 +153,7 @@ def validate_commit(src_id, dest_id):
def _validate_save(orig_dir, save_path, commit_id, tree_id):
global bup_cmd
# Check parent connectivity, etc.
- ex((b'git', b'--git-dir', b'get-dest', b'log', b'-n2', commit_id),
+ ex((b'git', b'-P', b'--git-dir', b'get-dest', b'log', b'-n2', commit_id),
stdin=DEVNULL)
rmrf(b'restore')
exr = verify_rcz((bup_cmd, b'-d', b'get-dest',
@@ -215,7 +215,7 @@ def validate_new_tagged_commit(tag_name, commit_id, tree_id, get_out):
wvpassne(commit_id, get_tag_id)
validate_tree(tree_id, tag_name + b':')
# Check parent connectivity, etc.
- ex((b'git', b'--git-dir', b'get-dest', b'log', b'-n2', tag_name),
+ ex((b'git', b'-P', b'--git-dir', b'get-dest', b'log', b'-n2', tag_name),
stdin=DEVNULL)

def _run_get(disposition, method, what):
--
2.47.3

[Rob Browning]

force-delete tmp first so the __pycache__ removal can't be thwarted by
anything troublesome in tmp.

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

GNUmakefile | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/GNUmakefile b/GNUmakefile
index edd97d64..6f0f4a1b 100644
--- a/GNUmakefile
+++ b/GNUmakefile
@@ -396,8 +396,8 @@ clean:
rm -rf $(bup_config_detritus)
rm -rf $(doc_clean_paths) $(clean_paths) .pytest_cache
rm -f $(generated_dependencies)
- find . -name __pycache__ -exec rm -rf {} +
if test -e test/tmp; then dev/force-delete test/tmp; fi
+ find . -name __pycache__ -exec rm -rf {} +
dev/configure-sampledata --clean

-include $(generated_dependencies)
--
2.47.3

[Rob Browning]

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

lib/bup/git.py | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/bup/git.py b/lib/bup/git.py
index be30abca..792c2964 100644
--- a/lib/bup/git.py
+++ b/lib/bup/git.py
@@ -1493,7 +1493,7 @@ class MissingObject(KeyError):
class WalkItem:
oid: bytes
name: bytes
- type: Union['blob', 'commit', 'tree']
+ type: Union[b'blob', b'commit', b'tree']
mode: int
data: Optional[bytes]

--
2.47.3

[Johannes Berg]

I think the subject is inverted, but I'm confused by the negatives :)

For "non-LocalRepos" we have a condition "true and ... and check()" so
the check will happen, now _only_ for non-LocalRepo?

johannes

[Rob Browning]

Johannes Berg <joha...@sipsolutions.net> writes:

> For "non-LocalRepos" we have a condition "true and ... and check()" so
> the check will happen, now _only_ for non-LocalRepo?

To be sure I understand, you're just saying that the subject is
wrong. If so, then agreed --- the change intends to do exactly what we
were doing, but only for remote repositories. I'll change it to

get_random_object: don't re-check existence for LocalRepos

Thanks

[Johannes Berg]

On Mon, 2025-11-10 at 11:44 -0600, Rob Browning wrote:
> Johannes Berg <joha...@sipsolutions.net> writes:
>
> > For "non-LocalRepos" we have a condition "true and ... and check()" so
> > the check will happen, now _only_ for non-LocalRepo?
>
> To be sure I understand, you're just saying that the subject is
> wrong.

Right. The logic is what you wanted :)

> If so, then agreed --- the change intends to do exactly what we
> were doing, but only for remote repositories. I'll change it to
>
> get_random_object: don't re-check existence for LocalRepos

Makes sense.

johannes