[FIX] odoobin: read the repository name from its git remote by brinkflew · Pull Request #179 · odoo-odev/odev

brinkflew · 2026-07-27T00:29:13Z

Description

odev run inside a git clone links it to the database under a name built from directory names —
f"{path.parent.name}/{path.name}" — even though it already holds the real path and has just
checked that it is a git repository. For a clone that does not follow the
<repositories>/<organization>/<repository> layout that name is wrong, and GitConnector.path
expands it back to a directory that does not exist:

repositories root:  ~/odoo/dev
clone:              ~/odoo/dev/tutorials
stored as:          dev/tutorials
resolved back to:   ~/odoo/dev/dev/tutorials

The wrong name is persisted, so every later command resolving the link looks in the duplicated
directory.

OdoobinProcess.additional_repositories passes the path to GitConnector, so the name is read
from the git remote — the only reliable source of truth — and the directory names become a
fallback rather than the value.
GitConnector resolves its name in _name_from_remote / _name_from_string / _fallback_name.
Reading the remote used to raise for a repository that has none and for a URL with fewer than two
segments, and it ignored remotes not named origin. All three now fall back to the name passed to
the connector instead of raising.
OdoobinCommand._guess_addons_paths no longer pins a repository whose directory is missing. It
preferred the stored repository over the current directory unconditionally, so a database already
linked under a wrong name could never be re-detected and silently ran without its custom addons.
It now warns, names the missing path and falls back to the current directory — which is what lets
an affected link repair itself on the next run.

Note for reviewers

Databases stored under a wrong name repair themselves the next time odev run is used from the
clone, but save_database_repository asks once whether to relink ("already linked to another
repository"). That prompt is the migration path. There is deliberately no upgrade script:
__validate_upgrade_script keys off the directory name, so it would need renaming on every rebase,
and the state is self-healing anyway.

GitConnector.path keeps expanding <repositories>/<name> with no flat-layout fallback. It also
feeds clone(), fix_corrupted() (which rmtrees it), remove, worktrees and
requirements_path, and a name-only match is ambiguous across organizations.

Tests

tests/tests/common/test_git_connector.py grows from name parsing only to real repositories built
with Repo.init + create_remote in a temporary directory: the name read from the remote overrides
the directory names (the regression guard for this issue), HTTPS and non-origin remotes, no remote
at all, an unparseable remote, an absolute path that is not a git repository, and the path
argument taking precedence over the conventional location. A second class covers the process end to
end — a flat clone directly under the repositories root now resolves to its own directory. Full test
suite passes.

Linked Issues

closes Repo dir duplicated #92

Compliance

I have read the contribution guide
I made sure the documentation is up-to-date both in doctrings and the docs directory
I have added or modified unit tests where necessary
I have added new libraries to the requirements.txt file, if any
I have incremented the version number according the versioning guide
The PR contains my changes only and no other external commit

🤖 Generated with Claude Code

https://claude.ai/code/session_01K8csZBrrBYp8oqH5paxTAm

`odev run` inside a git repository linked it to the database under a name built from directory names — `f"{path.parent.name}/{path.name}"` — even though it had the real path in hand and had just checked it was a git repository. For a clone that does not follow the `<repositories>/<organization>/<repository>` layout, that name is wrong, and `GitConnector.path` expands it back to a directory that does not exist: repositories root: ~/odoo/dev clone: ~/odoo/dev/tutorials stored as: dev/tutorials resolved to: ~/odoo/dev/dev/tutorials Pass the path to `GitConnector` so the name is read from the git remote, which is the only reliable source of truth, and keep the directory names as a fallback for repositories that have no remote. Two supporting changes: - `GitConnector` now resolves its name in `_name_from_remote` / `_name_from_string`. Reading the remote no longer raises when a repository has none, when its URL has fewer than two segments, or when the only remote is not named `origin`; all three fall back to the name passed to the connector. - `OdoobinCommand._guess_addons_paths` no longer pins a repository whose directory is missing. It used to prefer the stored repository over the current directory unconditionally, so a database already linked to a wrong name could never be re-detected and silently ran without its custom addons. It now warns and falls back to the current directory, which lets the link repair itself on the next run. Closes #92

Writing unit tests for helpers that had none surfaced five defects, all on code paths odev actually walks: - `PostgresConnector.columns_exist` returned an empty list when none of the requested columns existed, which is indistinguishable from all of them being present. A table created from an older definition therefore kept none of its new columns, since `CREATE TABLE IF NOT EXISTS` leaves an existing table alone and the missing-columns pass is the only thing that can migrate it. - `PostgresDatabase.tables` was a class attribute, so every database instance shared a single registry and tables from different databases collided on their name alone. - `string.quote` selected its delimiter with `max()` over the offsets of both quote characters, which picks the last one rather than the first and mis-quoted any string mixing them. The helper never escapes, so its docstring now says so. - `OdooVersion.__bool__` was always true: `module` is padded to `MIN_VERSION_LENGTH` and is therefore never an empty tuple. - `string.min_indent` raised on a text without any non-blank line, which `odev help` reaches through `dedent`. `float_to_hours` and `strip_styles` are broken too but are called nowhere in odev nor in the plugins; they are left alone and documented in the tests instead. Claude-Session: https://claude.ai/code/session_012tWyDuUYenE92nsBtG19Du

The suite sat at 64% with `.coveragerc` gating at 60%, and the gap was widest on pure logic the rest of the framework leans on. `odev/common/string.py` had no test module at all despite backing `odev help`, `odev history` and the database listing query. `GitWorktree` turns `git worktree list --porcelain` into the objects the whole fetch/pull/worktree family works with, and nothing exercised it. `OdooVersion` was tested for parsing only, while ordering is what actually picks a revision at runtime. Add test modules for the string helpers, the worktree parser and the datastore table preparation, plus ordering tests for versions. None of them need a network or a real repository. String and version helpers reach 100%, `common/postgres.py` 81% to 93% and the git connector 34% to 40%. Correct four things in the existing suite along the way: - The sudo tests shelled out to a real `sudo cat >> /etc/shadow`. The premise that the command fails only holds for an unprivileged user whose shell cannot open the redirection; a machine granting passwordless sudo runs it for real. Simulate the subprocess and the effective user instead, which also lets the elevation path be asserted rather than inferred. - `test_odev` left a command line behind in `sys.argv` for whichever test ran next. - `_patches` was a list defined on `OdevTestCase`, shared by every subclass through `cls._patches.append`, so each class tore down the patches of all the classes before it. - `PostgresTable` preparation is asserted against a mocked database: driving it against the live datastore made it depend on the connector's query cache, which DDL does not invalidate. Claude-Session: https://claude.ai/code/session_012tWyDuUYenE92nsBtG19Du

`is_odoo` checks that a database exists and then connects to it, and any process can drop it in between: `odev list` inspects every database in turn and would fail outright because one went away. A database that is gone is not an Odoo database, while anything else stays an error. The second check bypasses the query cache, since the cache is what claimed the database was still there. Claude-Session: https://claude.ai/code/session_012tWyDuUYenE92nsBtG19Du

`Odev` derived its name from the test mode alone, and everything it owns is named after it: the configuration file and the datastore database were fixed paths that any two instances had to share. It can now be given a name of its own. The setup script computed the destination of the `odev` symlink halfway through creating it, leaving no way to point it elsewhere; the decision moves to `link_path`. Both make the test suite able to run against resources of its own rather than against those of the user. Claude-Session: https://claude.ai/code/session_012tWyDuUYenE92nsBtG19Du

Everything the suite touched was named after a constant `odev-test`: the datastore database, the configuration file, the run directories and the databases created by the command tests. Two suites running at once shared all of it and destroyed each other's work — `odev delete --expression` removed the databases of the other run, dropping the datastore terminated its connections, and both wrote the same configuration file at once. A run now claims a sandbox named after itself and holds an exclusive lock on it for its whole life. The sandbox is removed when the session ends, including on `Ctrl+C` and on `SIGTERM`; the kernel drops the lock however the process dies, so a lock that can be taken marks leftovers the next run sweeps away. Nothing is written outside of it anymore: the configuration directory, the repositories and dumps directories, and the two symlinks the setup scripts create all point inside the sandbox, which also keeps the suite from repointing the `odev` command of the developer at whichever checkout it happens to run from. odev captures `SIGINT` around every query to cancel it rather than let it through, so an interrupt is recorded and acted upon at the next test boundary: pressing `Ctrl+C` stops the run without abandoning a connection mid-statement. Claude-Session: https://claude.ai/code/session_012tWyDuUYenE92nsBtG19Du

Both database context managers built a second, unconnected connector to close instead of the one they had connected, so `disconnect` did nothing and the connection stayed open until the garbage collector got to it. `ensure_connected` runs every database method inside its own block and those blocks nest — `is_odoo` opens one and then calls `table_exists`, which opens another — so this meant a fresh backend per call. Closing the right connector is not enough on its own: an inner block would close the connection the enclosing one is still using. The blocks are now reentrant and share a single connector, counted in the mixin so both classes get the same behaviour. The datastore keeps its connection instead of reopening it for each read: every command reads it and it lives as long as the process, which is not true of the databases odev walks through for `list` or `delete`. Over a full test suite run, the backends held at once drop from 42 to 3, and the suite goes from 55s to 34s. Claude-Session: https://claude.ai/code/session_012tWyDuUYenE92nsBtG19Du

Pin the behaviour a connection block is expected to have, since nothing failed loudly when it did not have it: leaving a block closes what it opened, a nested block joins the connection of the enclosing one rather than opening its own, repeated calls to decorated methods do not leave backends behind, and the datastore holds a single one throughout. The count is read from `pg_stat_activity`, which is what makes the third one a regression guard rather than a restatement of the code. Claude-Session: https://claude.ai/code/session_012tWyDuUYenE92nsBtG19Du

odev.common.logging configures logging on import, but logging.basicConfig is a no-op once the root logger has handlers: whether odev's handler gets installed depends on whether that import happens before or after pytest sets its own up, and importing the sandbox from conftest tips it. Combined with the capture added by #176 every record then reached the output twice, and a plain logger.info became a console.print that #177's run_hook test asserts on. The handler is dropped in pytest_configure so the suite no longer depends on import order. Also resolves the version command test, which this branch made namespace-agnostic while #175 rewrote it, and bumps the version to 4.31.2, one increment above the base branch.

…e ones #174 added a TestGithubConnectorRepositories class to this module, and both branches narrowed its imports to what they needed. Both sets of classes are kept and the imports are the union of the two. Also bumps the version to 4.31.3, one increment above the base branch.

brinkflew · 2026-08-06T21:43:27Z

Merge order: 4th of 4 — last

Important

Merge #174, #175 and #178, in that order, before this one. The base of this PR is now avs-tests-coverage-and-fixes (#178), not beta. GitHub retargets it as the ones above land.

beta has moved to 4.30.4 — #135, #177, #180 and #176 are already merged. The latest commit merges #178 in, resolves the conflict, and bumps the version to 4.31.3 (one increment above the base branch).

What the merge commit resolves

#174 added a TestGithubConnectorRepositories class to tests/tests/common/test_git_connector.py, the same module this branch extends with TestGitConnectorName, TestGitConnectorPath and TestOdoobinAdditionalRepositories. Both branches also narrowed the module imports to what they needed.

Both sets of classes are kept, and the imports are the union of the two — SimpleNamespace and the github exceptions from #174, shutil/tempfile/Path/Repo from here, and GitConnector, GithubConnector from the connectors module.

Queue

Order	PR	Branch	Status
1	#174	`beta-plugin-discovery-avs`	merge first
2	#175	`avs-update-warning`
3	#178	`avs-tests-coverage-and-fixes`
4	#179 (this one)	`avs-repository-path`	merge last

Once this lands, beta is at 4.31.3 and every one of the eight approved PRs is in.

Warning

GitHub Actions has not run in this repository since 2026-07-29, so the checks here are stale. The full suite was run locally against the exact merged tree of all four: 357 passed, pre-commit clean.

The merge-base changed after approval.

brinkflew · 2026-08-27T13:59:24Z

Merged

## Description `odev run` inside a git clone links it to the database under a name built from directory names — `f"{path.parent.name}/{path.name}"` — even though it already holds the real path and has just checked that it is a git repository. For a clone that does not follow the `<repositories>/<organization>/<repository>` layout that name is wrong, and `GitConnector.path` expands it back to a directory that does not exist: ``` repositories root: ~/odoo/dev clone: ~/odoo/dev/tutorials stored as: dev/tutorials resolved back to: ~/odoo/dev/dev/tutorials ``` The wrong name is persisted, so every later command resolving the link looks in the duplicated directory. - `OdoobinProcess.additional_repositories` passes the path to `GitConnector`, so the name is read from the git remote — the only reliable source of truth — and the directory names become a fallback rather than the value. - `GitConnector` resolves its name in `_name_from_remote` / `_name_from_string` / `_fallback_name`. Reading the remote used to raise for a repository that has none and for a URL with fewer than two segments, and it ignored remotes not named `origin`. All three now fall back to the name passed to the connector instead of raising. - `OdoobinCommand._guess_addons_paths` no longer pins a repository whose directory is missing. It preferred the stored repository over the current directory unconditionally, so a database already linked under a wrong name could never be re-detected and silently ran without its custom addons. It now warns, names the missing path and falls back to the current directory — which is what lets an affected link repair itself on the next run. ### Note for reviewers Databases stored under a wrong name repair themselves the next time `odev run` is used from the clone, but `save_database_repository` asks once whether to relink ("already linked to another repository"). That prompt is the migration path. There is deliberately no upgrade script: `__validate_upgrade_script` keys off the directory name, so it would need renaming on every rebase, and the state is self-healing anyway. `GitConnector.path` keeps expanding `<repositories>/<name>` with no flat-layout fallback. It also feeds `clone()`, `fix_corrupted()` (which `rmtree`s it), `remove`, `worktrees` and `requirements_path`, and a name-only match is ambiguous across organizations. ### Tests `tests/tests/common/test_git_connector.py` grows from name parsing only to real repositories built with `Repo.init` + `create_remote` in a temporary directory: the name read from the remote overrides the directory names (the regression guard for this issue), HTTPS and non-`origin` remotes, no remote at all, an unparseable remote, an absolute path that is not a git repository, and the `path` argument taking precedence over the conventional location. A second class covers the process end to end — a flat clone directly under the repositories root now resolves to its own directory. Full test suite passes. ## Linked Issues - closes #92 ## Compliance - [x] I have read the [contribution guide](../docs/CONTRIBUTING.md) - [x] I made sure the documentation is up-to-date both in doctrings and the `docs` directory - [x] I have added or modified unit tests where necessary - [x] I have added new libraries to the `requirements.txt` file, if any - [x] I have incremented the version number according the [versioning guide](../../docs/contributing/versioning.md) - [x] The PR contains **my changes only** and **no other external commit** 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01K8csZBrrBYp8oqH5paxTAm

brinkflew added 8 commits July 26, 2026 22:41

brinkflew requested a review from sea-odoo July 27, 2026 00:43

sea-odoo previously approved these changes Jul 30, 2026

View reviewed changes

Comment thread odev/common/connectors/git.py

brinkflew added 2 commits August 6, 2026 23:39

brinkflew dismissed sea-odoo’s stale review via d4176fa August 6, 2026 21:40

brinkflew changed the base branch from beta to avs-tests-coverage-and-fixes August 6, 2026 21:41

This was referenced Aug 6, 2026

[IMP] plugin: search, list and inspect odev plugins #174

Merged

[FIX] common: only report an available update when there is one #175

Closed

[FIX] common: close database connections, fix helpers, isolate the test suite #178

Merged

sea-odoo previously approved these changes Aug 27, 2026

View reviewed changes

brinkflew closed this Aug 27, 2026

brinkflew reopened this Aug 27, 2026

Base automatically changed from avs-tests-coverage-and-fixes to avs-update-warning August 27, 2026 13:33

brinkflew closed this Aug 27, 2026

brinkflew deleted the avs-repository-path branch August 27, 2026 14:15

Navigation Menu

Uh oh!

[FIX] odoobin: read the repository name from its git remote - #179

[FIX] odoobin: read the repository name from its git remote#179
brinkflew wants to merge 10 commits into
avs-update-warningfrom
avs-repository-path

brinkflew commented Jul 27, 2026

Uh oh!

Uh oh!

brinkflew commented Aug 6, 2026

Uh oh!

brinkflew commented Aug 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

brinkflew commented Jul 27, 2026

Description

Note for reviewers

Tests

Linked Issues

Compliance

Uh oh!

Uh oh!

brinkflew commented Aug 6, 2026

Merge order: 4th of 4 — last

What the merge commit resolves

Queue

Uh oh!

brinkflew commented Aug 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants