Rename Exponential backoff to Quadratic
#1815
Conversation
The sequence of backoff wait times used in `gitdb.db.loose` is quadratic rather than exponential, as discussed in: gitpython-developers#115 (comment) This corrects the variable name by making it more general, and the comment by having it explicitly describe the backoff as quadratic. This is conceptually related to GitoxideLabs/gitoxide#1815, but this is a non-breaking change, as no interfaces are affected: only a local variable and comment.
There was a problem hiding this comment.
Thanks a lot for your help with this!
The changes are perfectly mergeable, but I'd have to ask for a commit-refactor first to get the changelogs right. Please note that I typically have a second commit that says adapt to changes in <crate> to make code compile again.
| if cfg!(windows) { | ||
| // TODO(deps): Once `gix_path::env::shell()` is available, maybe do `shell().parent()?.join("bash.exe")` |
There was a problem hiding this comment.
I'm not sure I'm looking forward to it, actually. I'm not sure if shell() currently works properly in all the cases listed in #1761 where it reasonably should. But one case listed there that I expect it can and should be made to work--if it doesn't work already--is BusyBox-based MinGit. For that, there would not be a bash.exe in the same directory as the sh implementation shell() should find.
I look forward more to the possibility that gix-path--or some gix-* crate, since maybe this is actually in the ambit of gix-command--could report the locations of more executables.
sh should be preferred over bash for running shell scripts even when they are identical executables or one is a symlink to the other--because bash, and most other Bourne-style shells, behaves more POSIX-compatibly when invoked by the name sh. Yet finding and running bash is something that is often reasonable to do for specific reasons. It is a better choice in the gitoxide test suite, because the fixture scripts use bash-specific language features and they are not currently run in a way where their #! lines are automatically respected. There may even be some applications that need to use bash to run scripts that ideally should run in sh, just because they have been doing that for a long time already and users rely on it.
There are also various other executables that are often but not always available, and are relevant to Git, that are not found in the same directories as the Git for Windows sh.exe and bash.exe. An important case is perl. (MinGit does not have perl, but full Git for Windows installations do ship it, and I think it could be useful to find the one that comes with Git for Windows.)
More broadly, this could be part of a way of figuring out how to run interpreters specified in #! lines. Perhaps we should also have a function in gix-path, or somewhere. that takes a command name, to which "bash" could be passed.
Edit: Or maybe this is actually an alternative search behavior that should be achieved by opting into it through gix_command::Prepare.
There was a problem hiding this comment.
I see, thanks for sharing! Finding binaries isn't straightforward and having read this, I'd be glad if providing such functionality in gix-* crates would be minimised, strictly limited to strong use-cases or demands.
More broadly, this could be part of a way of figuring out how to run interpreters specified in
#!lines. Perhaps we should also have a function ingix-path, or somewhere. that takes a command name, to which"bash"could be passed.
If I remember correctly, on Windows this is already done via gix-command. But I don't recall if anything special is done to launch the command specified in the shebang. but think the implementation closely follows the one in Git.
There was a problem hiding this comment.
gix-command attempts to interpret shebangs, but I don't think it does so using the environment modifications Git for Windows puts in place. Traditionally Git for Windows did this in the git.exe shim but not the real git.exe executable that the shim points to. But since git-for-windows/git#2506 the real git.exe will detect when it looks like the customizations were not made and do them itself.
For PATH customizations especially, I think it might not be good to perform them or act by default as though they have been done. The kind of thing it does now, searching the preexisting PATH, seems better. Applications that use gix-* crates may run on Windows systems where Git for Windows is absent. Even when it is present, it is not clear that interpreters for hooks should in general be searched for first in Git for Windows related directories. There is also the subtlety that some executables found in directories Git for Windows adds to the PATH might malfunction in some uses if run without that environment set.
So while I think this may be useful functionality, it should probably continue not to be the default, and be some combination of opt-in and/or enabled automatically only for interpreters that are very likely to be wanted from Git for Windows. This might just be sh, but bash (which is alongside sh) and perl (which is not) are plausible others.
There was a problem hiding this comment.
Thanks for sharing, I wasn't aware that git.exe auto-modifies its PATH, something which indeed isn't done in gix-command. I also agree that it shouldn't be done by default, even though it might be useful to have at some point for opting in.
For now I assume that sh.exe used by Git will automatically allow all sibling commands to be called as well, which should make shell scripts on Windows more usable overall.
The `gix_utils::backoff::Exponential` type actually implemented quadratic, not exponential, backoff. This renames it from `Exponential` to `Quadratic`. In exponential backoff, delays are a fixed base, often 2, raised to a power of a number that increases by one with each attempt. When the number that increases by one with each attempt is the base, raised to a fixed power, that is quadratic backoff. The intended behavior here was quadratic, as implemented. For example, in the tests, `EXPECTED_TILL_SECOND` lists the values 1, 4, 9, 16, 25, 36, 49, 64, 81, 100, and so on, which are ascending squares. If they were an exponential sequence, then they would look like 1, 2, 4, 8, 16, 32, 64, 128, 256, 512, and so on. Thus, it is only the named that needed to be changed: the implementation was already as intended.
`Exponential` was renamed to `Quadratic` in `gix_utils::backoff`. This updates the use (and comment) in `gix-lock` accordingly.
- Add a TODO comment about changing `Exponential` to `Quadratic`. This cannot be done yet, but when upgrading dependencies, it will need to be done (since the rename is a breaking change). - Replace the TODO comment about `login_shell()` with one about how to use the value of `shell()` on Windows, and modify the last component, since GitoxideLabs#1758 replaced `login_shell()` with `shell()`. `shell()` returns a path to `sh`, and gix-testtools currently needs `bash` for fixtures. But the specific approach for finding `sh.exe` relative to the Git for Windows `git-core` directory also works for `bash.exe`.
Head branch was pushed to by a user without write access
This renames
backoff::Exponentialtobackoff::Quadraticand updates references to it, including in comments. For more details, see the commit message, and gitpython-developers/gitdb#115 (comment) where this change was suggested.I don't know if or how I should update the use in
gix-testtools, which depends on a version ofgix-utilsolder than what is tracked in the repository (#1510). The reference is in thespawn_git_daemonfunction:gitoxide/tests/tools/src/lib.rs
Line 222 in 38a0d9a
Edit: I added a comment to note it for later, when the dependencies of
gix-testtoolsare advanced to newer versions. While I was at it, I also changed another dependency-related comment, by rewriting and moving it to refer to a way of usingshell(), rather thanlogin_shell()which was removed in #1758.