summaryrefslogtreecommitdiffstats
path: root/bundle-uri.c (follow)
Commit message (Collapse)AuthorAgeFilesLines
* Merge branch 'ps/build-sign-compare'Junio C Hamano2024-12-231-0/+1
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Start working to make the codebase buildable with -Wsign-compare. * ps/build-sign-compare: t/helper: don't depend on implicit wraparound scalar: address -Wsign-compare warnings builtin/patch-id: fix type of `get_one_patchid()` builtin/blame: fix type of `length` variable when emitting object ID gpg-interface: address -Wsign-comparison warnings daemon: fix type of `max_connections` daemon: fix loops that have mismatching integer types global: trivial conversions to fix `-Wsign-compare` warnings pkt-line: fix -Wsign-compare warning on 32 bit platform csum-file: fix -Wsign-compare warning on 32-bit platform diff.h: fix index used to loop through unsigned integer config.mak.dev: drop `-Wno-sign-compare` global: mark code units that generate warnings with `-Wsign-compare` compat/win32: fix -Wsign-compare warning in "wWinMain()" compat/regex: explicitly ignore "-Wsign-compare" warnings git-compat-util: introduce macros to disable "-Wsign-compare" warnings
| * global: mark code units that generate warnings with `-Wsign-compare`Patrick Steinhardt2024-12-061-0/+1
| | | | | | | | | | | | | | | | | | Mark code units that generate warnings with `-Wsign-compare`. This allows for a structured approach to get rid of all such warnings over time in a way that can be easily measured. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | bundle: add bundle verification options typeJustin Tobler2024-11-281-2/+5
|/ | | | | | | | | | | | | | | | | | | When `unbundle()` is invoked, fsck verification may be configured by passing the `VERIFY_BUNDLE_FSCK` flag. This mechanism allows fsck checks on the bundle to be enabled or disabled entirely. To facilitate more fine-grained fsck configuration, additional context must be provided to `unbundle()`. Introduce the `unbundle_opts` type, which wraps the existing `verify_bundle_flags`, to facilitate future extension of `unbundle()` configuration. Also update `unbundle()` and its call sites to accept this new options type instead of the flags directly. The end behavior is functionally the same, but allows for the set of configurable options to be extended. This is leveraged in a subsequent commit to enable fsck message severity configuration. Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* bundle-uri: plug leak in unbundle_from_file()Toon Claes2024-10-101-5/+13
| | | | | | | | | | | | | | | | | | | | The function `unbundle_from_file()` has two memory leaks: - We do not release the `struct bundle_header header` when hitting errors because we return early without any cleanup. - We do not release the `struct strbuf bundle_ref` at all. Plug these leaks by creating a common exit path where both of these variables are released. While at it, refactor the code such that the variable assignments do not happen inside the conditional statement itself according to our coding style. Signed-off-by: Toon Claes <toon@iotcl.com> Acked-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'ps/environ-wo-the-repository'Junio C Hamano2024-09-231-1/+1
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Code clean-up. * ps/environ-wo-the-repository: (21 commits) environment: stop storing "core.notesRef" globally environment: stop storing "core.warnAmbiguousRefs" globally environment: stop storing "core.preferSymlinkRefs" globally environment: stop storing "core.logAllRefUpdates" globally refs: stop modifying global `log_all_ref_updates` variable branch: stop modifying `log_all_ref_updates` variable repo-settings: track defaults close to `struct repo_settings` repo-settings: split out declarations into a standalone header environment: guard state depending on a repository environment: reorder header to split out `the_repository`-free section environment: move `set_git_dir()` and related into setup layer environment: make `get_git_namespace()` self-contained environment: move object database functions into object layer config: make dependency on repo in `read_early_config()` explicit config: document `read_early_config()` and `read_very_early_config()` environment: make `get_git_work_tree()` accept a repository environment: make `get_graft_file()` accept a repository environment: make `get_index_file()` accept a repository environment: make `get_object_directory()` accept a repository environment: make `get_git_common_dir()` accept a repository ...
| * environment: move object database functions into object layerPatrick Steinhardt2024-09-121-1/+1
| | | | | | | | | | | | | | | | | | | | The `odb_mkstemp()` and `odb_pack_keep()` functions are quite clearly tied to the object store, but regardless of that they are located in "environment.c". Move them over, which also helps to get rid of dependencies on `the_repository` in the environment subsystem. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | fetch: add top-level trace2 regionsJosh Steadmon2024-08-231-0/+4
|/ | | | | | | | | | | | | At $DAYJOB we experienced some slow fetch operations and needed some additional data to help diagnose the issue. Add top-level trace2 regions for the various modes of operation of `git-fetch`. None of these regions are in recursive code, so any enclosed trace messages should only see their nesting level increase by one. Signed-off-by: Josh Steadmon <steadmon@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'xx/bundie-uri-fixes'Junio C Hamano2024-07-081-3/+3
|\ | | | | | | | | | | | | | | | | | | | | When bundleURI interface fetches multiple bundles, Git failed to take full advantage of all bundles and ended up slurping duplicated objects. * xx/bundie-uri-fixes: unbundle: extend object verification for fetches fetch-pack: expose fsckObjects configuration logic bundle-uri: verify oid before writing refs
| * unbundle: extend object verification for fetchesXing Xin2024-06-201-1/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The existing fetch.fsckObjects and transfer.fsckObjects configurations were not fully applied to bundle-involved fetches, including direct bundle fetches and bundle-uri enabled fetches. Furthermore, there was no object verification support for unbundle. This commit extends object verification support in `bundle.c:unbundle` by adding the `VERIFY_BUNDLE_FSCK` option to `verify_bundle_flags`. When this option is enabled, we append the `--fsck-objects` flag to `git-index-pack`. The `VERIFY_BUNDLE_FSCK` option is now used by bundle-involved fetches, where we use `fetch-pack.c:fetch_pack_fsck_objects` to determine whether to enable this option for `bundle.c:unbundle`, specifically in: - `transport.c:fetch_refs_from_bundle` for direct bundle fetches. - `bundle-uri.c:unbundle_from_file` for bundle-uri enabled fetches. This addition ensures a consistent logic for object verification during fetches. Tests have been added to confirm functionality in the scenarios mentioned above. Reviewed-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Xing Xin <xingxin.xx@bytedance.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * bundle-uri: verify oid before writing refsXing Xin2024-06-201-2/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When using the bundle-uri mechanism with a bundle list containing multiple interrelated bundles, we encountered a bug where tips from downloaded bundles were not discovered, thus resulting in rather slow clones. This was particularly problematic when employing the "creationTokens" heuristic. To reproduce this issue, consider a repository with a single branch "main" pointing to commit "A". Firstly, create a base bundle with: git bundle create base.bundle main Then, add a new commit "B" on top of "A", and create an incremental bundle for "main": git bundle create incr.bundle A..main Now, generate a bundle list with the following content: [bundle] version = 1 mode = all heuristic = creationToken [bundle "base"] uri = base.bundle creationToken = 1 [bundle "incr"] uri = incr.bundle creationToken = 2 A fresh clone with the bundle list above should result in a reference "refs/bundles/main" pointing to "B" in the new repository. However, git would still download everything from the server, as if it had fetched nothing locally. So why the "refs/bundles/main" is not discovered? After some digging I found that: 1. Bundles in bundle list are downloaded to local files via `bundle-uri.c:download_bundle_list` or via `bundle-uri.c:fetch_bundles_by_token` for the "creationToken" heuristic. 2. Each bundle is unbundled via `bundle-uri.c:unbundle_from_file`, which is called by `bundle-uri.c:unbundle_all_bundles` or called within `bundle-uri.c:fetch_bundles_by_token` for the "creationToken" heuristic. 3. To get all prerequisites of the bundle, the bundle header is read inside `bundle-uri.c:unbundle_from_file` to by calling `bundle.c:read_bundle_header`. 4. Then it calls `bundle.c:unbundle`, which calls `bundle.c:verify_bundle` to ensure the repository contains all the prerequisites. 5. `bundle.c:verify_bundle` calls `parse_object`, which eventually invokes `packfile.c:prepare_packed_git` or `packfile.c:reprepare_packed_git`, filling `raw_object_store->packed_git` and setting `packed_git_initialized`. 6. If `bundle.c:unbundle` succeeds, it writes refs via `refs.c:refs_update_ref` with `REF_SKIP_OID_VERIFICATION` set. Here bundle refs which can target arbitrary objects are written to the repository. 7. Finally, in `fetch-pack.c:do_fetch_pack_v2`, the functions `fetch-pack.c:mark_complete_and_common_ref` and `fetch-pack.c:mark_tips` are called with `OBJECT_INFO_QUICK` set to find local tips for negotiation. The `OBJECT_INFO_QUICK` flag prevents `packfile.c:reprepare_packed_git` from being called, resulting in failures to parse OIDs that reside only in the latest bundle. In the example above, when unbunding "incr.bundle", "base.pack" is added to `packed_git` due to prerequisites verification. However, "B" cannot be found for negotiation because it exists in "incr.pack", which is not included in `packed_git`. Fix the bug by removing `REF_SKIP_OID_VERIFICATION` flag when writing bundle refs. When `refs.c:refs_update_ref` is called to write the corresponding bundle refs, it triggers `refs.c:ref_transaction_commit`. This, in turn, invokes `refs.c:ref_transaction_prepare`, which calls `transaction_prepare` of the refs storage backend. For files backend, it is `files-backend.c:files_transaction_prepare`, and for reftable backend, it is `reftable-backend.c:reftable_be_transaction_prepare`. Both functions eventually call `object.c:parse_object`, which can invoke `packfile.c:reprepare_packed_git` to refresh `packed_git`. This ensures that bundle refs point to valid objects and that all tips from bundle refs are correctly parsed during subsequent negotiations. A set of negotiation-related tests for cloning with bundle-uri has been included to demonstrate that downloaded bundles are utilized to accelerate fetching. Additionally, another test has been added to show that bundles with incorrect headers, where refs point to non-existent objects, do not result in any bundle refs being created in the repository. Reviewed-by: Karthik Nayak <karthik.188@gmail.com> Reviewed-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Xing Xin <xingxin.xx@bytedance.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | global: introduce `USE_THE_REPOSITORY_VARIABLE` macroPatrick Steinhardt2024-06-141-0/+2
|/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Use of the `the_repository` variable is deprecated nowadays, and we slowly but steadily convert the codebase to not use it anymore. Instead, callers should be passing down the repository to work on via parameters. It is hard though to prove that a given code unit does not use this variable anymore. The most trivial case, merely demonstrating that there is no direct use of `the_repository`, is already a bit of a pain during code reviews as the reviewer needs to manually verify claims made by the patch author. The bigger problem though is that we have many interfaces that implicitly rely on `the_repository`. Introduce a new `USE_THE_REPOSITORY_VARIABLE` macro that allows code units to opt into usage of `the_repository`. The intent of this macro is to demonstrate that a certain code unit does not use this variable anymore, and to keep it from new dependencies on it in future changes, be it explicit or implicit For now, the macro only guards `the_repository` itself as well as `the_hash_algo`. There are many more known interfaces where we have an implicit dependency on `the_repository`, but those are not guarded at the current point in time. Over time though, we should start to add guards as required (or even better, just remove them). Define the macro as required in our code units. As expected, most of our code still relies on the global variable. Nearly all of our builtins rely on the variable as there is no way yet to pass `the_repository` to their entry point. For now, declare the macro in "biultin.h" to keep the required changes at least a little bit more contained. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* cocci: apply rules to rewrite callers of "refs" interfacesPatrick Steinhardt2024-05-071-5/+7
| | | | | | | | | Apply the rules that rewrite callers of "refs" interfaces to explicitly pass `struct ref_store`. The resulting patch has been applied with the `--whitespace=fix` option. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* treewide: remove unnecessary includes in source filesElijah Newren2023-12-261-1/+0
| | | | | | | | | | | | | | | | | | Each of these were checked with gcc -E -I. ${SOURCE_FILE} | grep ${HEADER_FILE} to ensure that removing the direct inclusion of the header actually resulted in that header no longer being included at all (i.e. that no other header pulled it in transitively). ...except for a few cases where we verified that although the header was brought in transitively, nothing from it was directly used in that source file. These cases were: * builtin/credential-cache.c * builtin/pull.c * builtin/send-pack.c Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* bundle-uri: mark unused parameters in callbacksJeff King2023-08-301-3/+3
| | | | | | | | | | | The first hunk is similar to 02c3c59e62 (hashmap: mark unused callback parameters, 2022-08-19), but was added after that commit. The other two are used with for_all_bundles_in_list(), but don't use their void data pointer. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'gc/config-context'Junio C Hamano2023-07-061-2/+7
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Reduce reliance on a global state in the config reading API. * gc/config-context: config: pass source to config_parser_event_fn_t config: add kvi.path, use it to evaluate includes config.c: remove config_reader from configsets config: pass kvi to die_bad_number() trace2: plumb config kvi config.c: pass ctx with CLI config config: pass ctx with config files config.c: pass ctx in configsets config: add ctx arg to config_fn_t urlmatch.h: use config_fn_t type config: inline git_color_default_config
| * config: pass ctx with config filesGlen Choo2023-06-281-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Pass config_context to config_callbacks when parsing config files. To provide the .kvi member, refactor out the configset logic that caches "struct config_source" and "enum config_scope" as a "struct key_value_info". Make the "enum config_scope" available to the config file machinery by plumbing an additional arg through git_config_from_file_with_options(). We do not exercise ctx yet because the remaining current_config_*() callers may be used with config_with_options(), which may read config from parameters, but parameters don't pass ctx yet. Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * config: add ctx arg to config_fn_tGlen Choo2023-06-281-2/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add a new "const struct config_context *ctx" arg to config_fn_t to hold additional information about the config iteration operation. config_context has a "struct key_value_info kvi" member that holds metadata about the config source being read (e.g. what kind of config source it is, the filename, etc). In this series, we're only interested in .kvi, so we could have just used "struct key_value_info" as an arg, but config_context makes it possible to add/adjust members in the future without changing the config_fn_t signature. We could also consider other ways of organizing the args (e.g. moving the config name and value into config_context or key_value_info), but in my experiments, the incremental benefit doesn't justify the added complexity (e.g. a config_fn_t will sometimes invoke another config_fn_t but with a different config value). In subsequent commits, the .kvi member will replace the global "struct config_reader" in config.c, making config iteration a global-free operation. It requires much more work for the machinery to provide meaningful values of .kvi, so for now, merely change the signature and call sites, pass NULL as a placeholder value, and don't rely on the arg in any meaningful way. Most of the changes are performed by contrib/coccinelle/config_fn_ctx.pending.cocci, which, for every config_fn_t: - Modifies the signature to accept "const struct config_context *ctx" - Passes "ctx" to any inner config_fn_t, if needed - Adds UNUSED attributes to "ctx", if needed Most config_fn_t instances are easily identified by seeing if they are called by the various config functions. Most of the remaining ones are manually named in the .cocci patch. Manual cleanups are still needed, but the majority of it is trivial; it's either adjusting config_fn_t that the .cocci patch didn't catch, or adding forward declarations of "struct config_context ctx" to make the signatures make sense. The non-trivial changes are in cases where we are invoking a config_fn_t outside of config machinery, and we now need to decide what value of "ctx" to pass. These cases are: - trace2/tr2_cfg.c:tr2_cfg_set_fl() This is indirectly called by git_config_set() so that the trace2 machinery can notice the new config values and update its settings using the tr2 config parsing function, i.e. tr2_cfg_cb(). - builtin/checkout.c:checkout_main() This calls git_xmerge_config() as a shorthand for parsing a CLI arg. This might be worth refactoring away in the future, since git_xmerge_config() can call git_default_config(), which can do much more than just parsing. Handle them by creating a KVI_INIT macro that initializes "struct key_value_info" to a reasonable default, and use that to construct the "ctx" arg. Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | object-store-ll.h: split this header out of object-store.hElijah Newren2023-06-211-1/+1
|/ | | | | | | | | | | | | | | | | The vast majority of files including object-store.h did not need dir.h nor khash.h. Split the header into two files, and let most just depend upon object-store-ll.h, while letting the two callers that need it depend on the full object-store.h. After this patch: $ git grep -h include..object-store | sort | uniq -c 2 #include "object-store.h" 129 #include "object-store-ll.h" Diff best viewed with `--color-moved`. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'en/header-split-cache-h-part-2'Junio C Hamano2023-05-101-1/+2
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | More header clean-up. * en/header-split-cache-h-part-2: (22 commits) reftable: ensure git-compat-util.h is the first (indirect) include diff.h: reduce unnecessary includes object-store.h: reduce unnecessary includes commit.h: reduce unnecessary includes fsmonitor: reduce includes of cache.h cache.h: remove unnecessary headers treewide: remove cache.h inclusion due to previous changes cache,tree: move basic name compare functions from read-cache to tree cache,tree: move cmp_cache_name_compare from tree.[ch] to read-cache.c hash-ll.h: split out of hash.h to remove dependency on repository.h tree-diff.c: move S_DIFFTREE_IFXMIN_NEQ define from cache.h dir.h: move DTYPE defines from cache.h versioncmp.h: move declarations for versioncmp.c functions from cache.h ws.h: move declarations for ws.c functions from cache.h match-trees.h: move declarations for match-trees.c functions from cache.h pkt-line.h: move declarations for pkt-line.c functions from cache.h base85.h: move declarations for base85.c functions from cache.h copy.h: move declarations for copy.c functions from cache.h server-info.h: move declarations for server-info.c functions from cache.h packfile.h: move pack_window and pack_entry from cache.h ...
| * treewide: remove cache.h inclusion due to previous changesElijah Newren2023-04-241-1/+1
| | | | | | | | | | Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * copy.h: move declarations for copy.c functions from cache.hElijah Newren2023-04-241-0/+1
| | | | | | | | | | Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | fetch_bundle_uri(): drop pointless NULL checkJeff King2023-04-241-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We check if "uri" is NULL, but it cannot be since we'd have segfaulted earlier in the function when we unconditionally called xstrdup() on it. In theory we might want to soften that xstrdup() to handle this case, but even before the code which added it via c23f592117 (bundle-uri: fetch a list of bundles, 2022-10-12), we'd have fed NULL to fetch_bundle_uri_internal(), which would also segfault. The extra check isn't hurting anything, but it does cause Coverity to complain, and it may mislead somebody reading the code into thinking that a NULL uri is something we're prepared to handle. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'ds/fetch-bundle-uri-with-all'Junio C Hamano2023-04-061-0/+9
|\ \ | |/ |/| | | | | | | | | | | "git fetch --all" does not have to download and handle the same bundleURI over and over, which has been corrected. * ds/fetch-bundle-uri-with-all: fetch: download bundles once, even with --all
| * fetch: download bundles once, even with --allDerrick Stolee2023-03-311-0/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When fetch.bundleURI is set, 'git fetch' downloads bundles from the given bundle URI before fetching from the specified remote. However, when using non-file remotes, 'git fetch --all' will launch 'git fetch' subprocesses which then read fetch.bundleURI and fetch the bundle list again. We do not expect the bundle list to have new information during these multiple runs, so avoid these extra calls by un-setting fetch.bundleURI in the subprocess arguments. Be careful to skip fetching bundles for the empty bundle string. Fetching bundles from the empty list presents some interesting test failures. Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | environment.h: move declarations for environment.c functions from cache.hElijah Newren2023-03-211-0/+1
| | | | | | | | | | Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | treewide: be explicit about dependence on gettext.hElijah Newren2023-03-211-0/+1
|/ | | | | | | | | | | | | | Dozens of files made use of gettext functions, without explicitly including gettext.h. This made it more difficult to find which files could remove a dependence on cache.h. Make C files explicitly include gettext.h if they are using it. However, while compat/fsmonitor/fsm-ipc-darwin.c should also gain an include of gettext.h, it was left out to avoid conflicting with an in-flight topic. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* serve: use repository pointer to get configJeff King2023-02-241-1/+1
| | | | | | | | | | | | | | | A few of the v2 "serve" callbacks ignore their repository parameter and read config using the_repository (either directly or implicitly by calling wrapper functions). This isn't a bug since the server code only handles a single main repository anyway (and indeed, if you look at the callers, these repository parameters will always be the_repository). But in the long run we want to get rid of the_repository, so let's take a tiny step in that direction. As a bonus, this silences some -Wunused-parameter warnings. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'ds/bundle-uri-5'Junio C Hamano2023-02-161-3/+246
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The bundle-URI subsystem adds support for creation-token heuristics to help incremental fetches. * ds/bundle-uri-5: bundle-uri: test missing bundles with heuristic bundle-uri: store fetch.bundleCreationToken fetch: fetch from an external bundle URI bundle-uri: drop bundle.flag from design doc clone: set fetch.bundleURI if appropriate bundle-uri: download in creationToken order bundle-uri: parse bundle.<id>.creationToken values bundle-uri: parse bundle.heuristic=creationToken t5558: add tests for creationToken heuristic bundle: verify using check_connected() bundle: test unbundling with incomplete history
| * bundle-uri: store fetch.bundleCreationTokenDerrick Stolee2023-01-311-2/+46
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When a bundle list specifies the "creationToken" heuristic, the Git client downloads the list and then starts downloading bundles in descending creationToken order. This process stops as soon as all downloaded bundles can be applied to the repository (because all required commits are present in the repository or in the downloaded bundles). When checking the same bundle list twice, this strategy requires downloading the bundle with the maximum creationToken again, which is wasteful. The creationToken heuristic promises that the client will not have a use for that bundle if its creationToken value is at most the previous creationToken value. To prevent these wasteful downloads, create a fetch.bundleCreationToken config setting that the Git client sets after downloading bundles. This value allows skipping that maximum bundle download when this config value is the same value (or larger). To test that this works correctly, we can insert some "duplicate" fetches into existing tests and demonstrate that only the bundle list is downloaded. The previous logic for downloading bundles by creationToken worked even if the bundle list was empty, but now we have logic that depends on the first entry of the list. Terminate early in the (non-sensical) case of an empty bundle list. Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * clone: set fetch.bundleURI if appropriateDerrick Stolee2023-01-311-1/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Bundle providers may organize their bundle lists in a way that is intended to improve incremental fetches, not just initial clones. However, they do need to state that they have organized with that in mind, or else the client will not expect to save time by downloading bundles after the initial clone. This is done by specifying a bundle.heuristic value. There are two types of bundle lists: those at a static URI and those that are advertised from a Git remote over protocol v2. The new fetch.bundleURI config value applies for static bundle URIs that are not advertised over protocol v2. If the user specifies a static URI via 'git clone --bundle-uri', then Git can set this config as a reminder for future 'git fetch' operations to check the bundle list before connecting to the remote(s). For lists provided over protocol v2, we will want to take a different approach and create a property of the remote itself by creating a remote.<id>.* type config key. That is not implemented in this change. Later changes will update 'git fetch' to consume this option. Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * bundle-uri: download in creationToken orderDerrick Stolee2023-01-311-2/+154
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The creationToken heuristic provides an ordering on the bundles advertised by a bundle list. Teach the Git client to download bundles differently when this heuristic is advertised. The bundles in the list are sorted by their advertised creationToken values, then downloaded in decreasing order. This avoids the previous strategy of downloading bundles in an arbitrary order and attempting to apply them (likely failing in the case of required commits) until discovering the order through attempted unbundling. During a fresh 'git clone', it may make sense to download the bundles in increasing order, since that would prevent the need to attempt unbundling a bundle with required commits that do not exist in our empty object store. The cost of testing an unbundle is quite low, and instead the chosen order is optimizing for a future bundle download during a 'git fetch' operation with a non-empty object store. Since the Git client continues fetching from the Git remote after downloading and unbundling bundles, the client's object store can be ahead of the bundle provider's object store. The next time it attempts to download from the bundle list, it makes most sense to download only the most-recent bundles until all tips successfully unbundle. The strategy implemented here provides that short-circuit where the client downloads a minimal set of bundles. However, we are not satisfied by the naive approach of downloading bundles until one successfully unbundles, expecting the earlier bundles to successfully unbundle now. The example repository in t5558 demonstrates this well: ---------------- bundle-4 4 / \ ----|---|------- bundle-3 | | | 3 | | ----|---|------- bundle-2 | | 2 | | | ----|---|------- bundle-1 \ / 1 | (previous commits) In this repository, if we already have the objects for bundle-1 and then try to fetch from this list, the naive approach will fail. bundle-4 requires both bundle-3 and bundle-2, though bundle-3 will successfully unbundle without bundle-2. Thus, the algorithm needs to keep this in mind. A later implementation detail will store the maximum creationToken seen during such a bundle download, and the client will avoid downloading a bundle unless its creationToken is strictly greater than that stored value. For now, if the client seeks to download from an identical bundle list since its previous download, it will download the most-recent bundle then stop since its required commits are already in the object store. Add tests that exercise this behavior, but we will expand upon these tests when incremental downloads during 'git fetch' make use of creationToken values. Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * bundle-uri: parse bundle.<id>.creationToken valuesDerrick Stolee2023-01-311-0/+10
| | | | | | | | | | | | | | | | | | | | | | | | | | The previous change taught Git to parse the bundle.heuristic value, especially when its value is "creationToken". Now, teach Git to parse the bundle.<id>.creationToken values on each bundle in a bundle list. Before implementing any logic based on creationToken values for the creationToken heuristic, parse and print these values for testing purposes. Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * bundle-uri: parse bundle.heuristic=creationTokenDerrick Stolee2023-01-311-0/+34
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The bundle.heuristic value communicates that the bundle list is organized to make use of the bundle.<id>.creationToken values that may be provided in the bundle list. Those values will create a total order on the bundles, allowing the Git client to download them in a specific order and even remember previously-downloaded bundles by storing the maximum creation token value. Before implementing any logic that parses or uses the bundle.<id>.creationToken values, teach Git to parse the bundle.heuristic value from a bundle list. We can use 'test-tool bundle-uri' to print the heuristic value and verify that the parsing works correctly. As an extra precaution, create the internal 'heuristics' array to be a list of (enum, string) pairs so we can iterate through the array entries carefully, regardless of the enum values. Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | convert trivial uses of strncmp() to starts_with()Jeff King2023-01-081-1/+1
|/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | It's more readable to use starts_with() instead of strncmp() to match a prefix, as the latter requires a manually-computed length, and has the funny "matching is zero" return value common to cmp functions. This patch converts several cases which were found with: git grep 'strncmp(.*, [0-9]*)' But note that it doesn't convert all such cases. There are several where the magic length number is repeated elsewhere in the code, like: /* handle "buf" which isn't NUL-terminated and might be too small */ if (len >= 3 && !strncmp(buf, "foo", 3)) or: /* exact match for "foo", but within a larger string */ if (end - buf == 3 && !strncmp(buf, "foo", 3)) While it would not produce the wrong outcome to use starts_with() in these cases, we'd still be left with one instance of "3". We're better to leave them for now, as the repeated "3" makes it clear that the two are linked (there may be other refactorings that handle both, but they're out of scope for this patch). A few things to note while reading the patch: - all cases but one are trying to match, and so lose the extra "!". The case in the first hunk of urlmatch.c is not-matching, and hence gains a "!". - the case in remote-fd.c is matching the beginning of "connect foo", but we never look at str+8 to parse the "foo" part (which would make this a candidate for skip_prefix(), not starts_with()). This seems at first glance like a bug, but is a limitation of how remote-fd works. - the second hunk in urlmatch.c shows some cases adjacent to other strncmp() calls that are left. These are of the "exact match within a larger string" type, as described above. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* bundle-uri: download bundles from an advertised listDerrick Stolee2022-12-251-0/+21
| | | | | | | | | | | | | | | | | | | | | | | The logic in fetch_bundle_uri() is useful for the --bundle-uri option of 'git clone', but is not helpful when the clone operation discovers a list of URIs from the bundle-uri protocol v2 command. To actually download and unbundle the advertised bundles, we need a different mechanism. Create the new fetch_bundle_list() method which is very similar to fetch_bundle_uri() except that it relies on download_bundle_list() instead of fetch_bundle_uri_internal(). The download_bundle_list() method will recursively call fetch_bundle_uri_internal() if any of the advertised URIs serve a bundle list instead of a bundle. This will also follow the bundle.list.mode setting from the input list: "any" will download only one such URI while "all" will download data from all of the URIs. In an identical way to fetch_bundle_uri(), the bundles are unbundled after all of the bundle lists have been expanded and all necessary URIs. Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* bundle-uri: allow relative URLs in bundle listsDerrick Stolee2022-12-251-1/+15
| | | | | | | | | | | | | | | | | | Bundle providers may want to distribute that data across multiple CDNs. This might require a change in the base URI, all the way to the domain name. If all bundles require an absolute URI in their 'uri' value, then every push to a CDN would require altering the table of contents to match the expected domain and exact location within it. Allow a bundle list to specify a relative URI for the bundles. This URI is based on where the client received the bundle list. For a list provided in the 'bundle-uri' protocol v2 command, the Git remote URI is the base URI. Otherwise, the bundle list was provided from an HTTP URI not using the Git protocol, and that URI is the base URI. This allows easier distribution of bundle data. Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* bundle-uri: serve bundle.* keys from configDerrick Stolee2022-12-251-1/+15
| | | | | | | | | | | | | | Implement the "bundle-uri" protocol v2 capability by populating the key=value packet lines from the local Git config. The list of bundles is provided from the keys beginning with "bundle.". In the future, we may want to filter this list to be more specific to the exact known keys that the server intends to share, but for flexibility at the moment we will assume that the config values are well-formed. Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* protocol v2: add server-side "bundle-uri" skeletonÆvar Arnfjörð Bjarmason2022-12-251-0/+36
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add a skeleton server-side implementation of a new "bundle-uri" command to protocol v2. This will allow conforming clients to optionally seed their initial clones or incremental fetches from URLs containing "*.bundle" files created with "git bundle create". This change only performs the basic boilerplate of advertising a new protocol v2 capability. The new 'bundle-uri' capability allows a client to request a list of bundles. Right now, the server only returns a flush packet, which corresponds to an empty advertisement. The bundle.* config namespace describes which key-value pairs will be communicated across this interface in future updates. The critical bit right now is that the new boolean uploadPack.adverstiseBundleURIs config value signals whether or not this capability should be advertised at all. An earlier version of this patch [1] used a different transfer format than the "key=value" pairs in the current implementation. The change was made to unify the protocol v2 command with the bundle lists provided by independent bundle servers. Further, the standard allows for the server to advertise a URI that contains a bundle list. This allows users automatically discovering bundle providers that are loosely associated with the origin server, but without the origin server knowing exactly which bundles are currently available. [1] https://lore.kernel.org/git/RFC-patch-v2-01.13-2fc87ce092b-20220311T155841Z-avarab@gmail.com/ The very-deep headings needed to be modified to stop at level 4 due to documentation build issues. These were not recognized in earlier builds since the file was previously in the Documentation/technical/ directory and was built in a different way. With its current location, the heavily-nested details were causing build issues and they are now replaced with a bulletted list of details. Co-authored-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* bundle-uri: suppress stderr from remote-httpsDerrick Stolee2022-10-121-0/+1
| | | | | | | | | | | | | | | | | | When downloading bundles from a git-remote-https subprocess, the bundle URI logic wants to be opportunistic and download as much as possible and work with what did succeed. This is particularly important in the "any" mode, where any single bundle success will work. If the URI is not available, the git-remote-https process will die() with a "fatal:" error message, even though that error is not actually fatal to the super process. Since stderr is passed through, it looks like a fatal error to the user. Suppress stderr to avoid these errors from bubbling to the surface. The bundle URI API adds its own warning() messages on these failures. Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* bundle-uri: quiet failed unbundlingsDerrick Stolee2022-10-121-1/+2
| | | | | | | | | | | | | | | | | | | | When downloading a list of bundles in "all" mode, Git has no understanding of the dependencies between the bundles. Git attempts to unbundle the bundles in some order, but some may not pass the verify_bundle() step because of missing prerequisites. This is passed as error messages to the user, even when they eventually succeed in later attempts after their dependent bundles are unbundled. Add a new VERIFY_BUNDLE_QUIET flag to verify_bundle() that avoids the error messages from the missing prerequisite commits. The method still returns the number of missing prerequisit commits, allowing callers to unbundle() to notice that the bundle failed to apply. Use this flag in bundle-uri.c and test that the messages go away for 'git clone --bundle-uri' commands. Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* bundle: add flags to verify_bundle()Derrick Stolee2022-10-121-1/+6
| | | | | | | | | | The verify_bundle() method has a 'verbose' option, but we will want to extend this method to have more granular control over its output. First, replace this 'verbose' option with a new 'flags' option with a single possible value: VERIFY_BUNDLE_VERBOSE. Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* bundle-uri: fetch a list of bundlesDerrick Stolee2022-10-121-16/+187
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When the content at a given bundle URI is not understood as a bundle (based on inspecting the initial content), then Git currently gives up and ignores that content. Independent bundle providers may want to split up the bundle content into multiple bundles, but still make them available from a single URI. Teach Git to attempt parsing the bundle URI content as a Git config file providing the key=value pairs for a bundle list. Git then looks at the mode of the list to see if ANY single bundle is sufficient or if ALL bundles are required. The content at the selected URIs are downloaded and the content is inspected again, creating a recursive process. To guard the recursion against malformed or malicious content, limit the recursion depth to a reasonable four for now. This can be converted to a configured value in the future if necessary. The value of four is twice as high as expected to be useful (a bundle list is unlikely to point to more bundle lists). To test this scenario, create an interesting bundle topology where three incremental bundles are built on top of a single full bundle. By using a merge commit, the two middle bundles are "independent" in that they do not require each other in order to unbundle themselves. They each only need the base bundle. The bundle containing the merge commit requires both of the middle bundles, though. This leads to some interesting decisions when unbundling, especially when we later implement heuristics that promote downloading bundles until the prerequisite commits are satisfied. Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* bundle-uri: limit recursion depth for bundle listsDerrick Stolee2022-10-121-1/+20
| | | | | | | | | | | | | | | | The next change will start allowing us to parse bundle lists that are downloaded from a provided bundle URI. Those lists might point to other lists, which could proceed to an arbitrary depth (and even create cycles). Restructure fetch_bundle_uri() to have an internal version that has a recursion depth. Compare that to a new max_bundle_uri_depth constant that is twice as high as we expect this depth to be for any legitimate use of bundle list linking. We can consider making max_bundle_uri_depth a configurable value if there is demonstrated value in the future. Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* bundle-uri: parse bundle list in config formatDerrick Stolee2022-10-121-0/+27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When a bundle provider wants to operate independently from a Git remote, they want to provide a single, consistent URI that users can use in their 'git clone --bundle-uri' commands. At this point, the Git client expects that URI to be a single bundle that can be unbundled and used to bootstrap the rest of the clone from the Git server. This single bundle cannot be re-used to assist with future incremental fetches. To allow for the incremental fetch case, teach Git to understand a bundle list that could be advertised at an independent bundle URI. Such a bundle list is likely to be inspected by human readers, even if only by the bundle provider creating the list. For this reason, we can take our expected "key=value" pairs and instead format them using Git config format. Create bundle_uri_parse_config_format() to parse a file in config format and convert that into a 'struct bundle_list' filled with its understanding of the contents. Be careful to use error_action CONFIG_ERROR_ERROR when calling git_config_from_file_with_options() because the default action for git_config_from_file() is to die() on a parsing error. The current warning isn't particularly helpful if it arises to a user, but it will be made more verbose at a higher layer later. Update 'test-tool bundle-uri' to take this config file format as input. It uses a filename instead of stdin because there is no existing way to parse a FILE pointer in the config machinery. Using git_config_from_mem() is overly complicated and more likely to introduce bugs than this simpler version. Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* bundle-uri: unit test "key=value" parsingÆvar Arnfjörð Bjarmason2022-10-121-0/+33
| | | | | | | | | | | | | | | | | | | Create a new 'test-tool bundle-uri' test helper. This helper will assist in testing logic deep in the bundle URI feature. This change introduces the 'parse-key-values' subcommand, which parses an input file as a list of lines. These are fed into bundle_uri_parse_line() to test how we construct a 'struct bundle_list' from that data. The list is then output to stdout as if the key-value pairs were a Git config file. We use an input file instead of stdin because of a future change to parse in config-file format that works better as an input file. Co-authored-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* bundle-uri: create "key=value" line parsingÆvar Arnfjörð Bjarmason2022-10-121-1/+26
| | | | | | | | | | | | | | | When advertising a bundle list over Git's protocol v2, we will use packet lines. Each line will be of the form "key=value" representing a bundle list. Connect the API necessary for Git's transport to the key-value pair parsing created in the previous change. We are not currently implementing this protocol v2 functionality, but instead preparing to expose this parsing to be unit-testable. Co-authored-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* bundle-uri: create base key-value pair parsingDerrick Stolee2022-10-121-0/+76
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | There will be two primary ways to advertise a bundle list: as a list of packet lines in Git's protocol v2 and as a config file served from a bundle URI. Both of these fundamentally use a list of key-value pairs. We will use the same set of key-value pairs across these formats. Create a new bundle_list_update() method that is currently unusued, but will be used in the next change. It inspects each key to see if it is understood and then applies it to the given bundle_list. Here are the keys that we teach Git to understand: * bundle.version: This value should be an integer. Git currently understands only version 1 and will ignore the list if the version is any other value. This version can be increased in the future if we need to add new keys that Git should not ignore. We can add new "heuristic" keys without incrementing the version. * bundle.mode: This value should be one of "all" or "any". If this mode is not understood, then Git will ignore the list. This mode indicates whether Git needs all of the bundle list items to make a complete view of the content or if any single item is sufficient. The rest of the keys use a bundle identifier "<id>" as part of the key name. Keys using the same "<id>" describe a single bundle list item. * bundle.<id>.uri: This stores the URI of the bundle item. This currently is expected to be an absolute URI, but will be relaxed to be a relative URI in the future. While parsing, return an error if a URI key is repeated, since we can make that restriction with bundle lists. Make the git_parse_int() method global so we can parse the integer version value carefully. Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* bundle-uri: create bundle_list struct and helpersDerrick Stolee2022-10-121-0/+60
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | It will likely be rare where a user uses a single bundle URI and expects that URI to point to a bundle. Instead, that URI will likely be a list of bundles provided in some format. Alternatively, the Git server could advertise a list of bundles. In anticipation of these two ways of advertising multiple bundles, create a data structure that represents such a list. This will be populated using a common API, but for now focus on what data can be represented. Each list contains a number of remote_bundle_info structs. These contain an 'id' that is used to uniquely identify them in the list, and also a 'uri' that contains the location of its data. Finally, there is a strbuf containing the filename used when Git downloads the contents to disk. The list itself stores these remote_bundle_info structs in a hashtable using 'id' as the key. The order of the structs in the input is considered unimportant, but future modifications to the format and these data structures will place ordering possibilities on the set. The list also has a few "global" properties, including the version (used when parsing the list) and the mode. The mode is one of these two options: 1. BUNDLE_MODE_ALL: all listed URIs are intended to be combined together. The client should download all of the advertised data to have a complete copy of the data. 2. BUNDLE_MODE_ANY: any one listed item is sufficient to have a complete copy of the data. The client can choose arbitrarily from these options. In the future, the client may use pings to find the closest URI among geodistributed replicas, or use some other heuristic information added to the format. This API is currently unused, but will soon be expanded with parsing logic and then be consumed by the bundle URI download logic. Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* bundle-uri: use plain string in find_temp_filename()Derrick Stolee2022-10-121-12/+16
| | | | | | | | | | | | | | | | | | The find_temp_filename() method was created in 53a50892be2 (bundle-uri: create basic file-copy logic, 2022-08-09) and uses odb_mkstemp() to create a temporary filename. The odb_mkstemp() method uses a strbuf in its interface, but we do not need to continue carrying a strbuf throughout the bundle URI code. Convert the find_temp_filename() method to use a 'char *' and modify its only caller. This makes sense that we don't actually need to modify this filename directly later, so using a strbuf is overkill. This change will simplify the data structure for tracking a bundle list to use plain strings instead of strbufs. Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* bundle-uri: add support for http(s):// and file://Derrick Stolee2022-08-101-3/+67
| | | | | | | | | | | | | | The previous change created the 'git clone --bundle-uri=<uri>' option. Currently, <uri> must be a filename. Update copy_uri_to_file() to first inspect the URI for an HTTP(S) prefix and use git-remote-https as the way to download the data at that URI. Otherwise, check to see if file:// is present and modify the prefix accordingly. Reviewed-by: Josh Steadmon <steadmon@google.com> Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>