Outdated broken links from documentation into code

[EliahKagan]

Current behavior 😯

There were three broken links found while working on #1624 that I didn't include fixes for in 65f69f7 because there was no clear best way to fix them. For two of them, which will probably require updating planning information. I am not at all sure what should be done, though I've tried to suggest broad possibilities. Those are the ones that motivate this issue. For the third, I'm pretty sure what the correct fix is and I'll go ahead and open a PR for it, but I've included it here as well so it can be easily reviewed.

• crate-status.md:232 (#1631)
• general-tasks.md:19 (#1631)
• gix-diff/src/tree/function.rs:27 (#1628)

Please note that the broken links described in this issue were effectively unaffected by the change of Byron/gitoxide to GitoxideLabs/gitoxide in 64ff0a7: although that part of their URLs changed, they were broken in exactly the same way both before and after that change to their URLs.

In crate-status.md

That link is a 404, because it specifies a gix-pack path at a commit before git-pack was renamed to gix-pack:

gitoxide/crate-status.md

Line 232 in 795962b

* [ ] [fast pack traversal works with ref-deltas](https://github.com/GitoxideLabs/gitoxide/blob/8f9a55bb31af32b266d7c53426bc925361a627b2/gix-pack/src/cache/delta/from_offsets.rs#L101-L105)

Changing the path back to git-path gives:

gitoxide/git-pack/src/cache/delta/from_offsets.rs

Lines 101 to 105 in 8f9a55b

	// TODO: ref-deltas can also point to objects (by offset) which we haven't seen yet, so this should really be
	// handled maybe by delaying those? Add-Child needs things ordered ascending though, so it's not trivial to do
	// and probably requires tham offset-rewriting.
	// Note that some packs created by libgit2 generally look like that, see the 'index-bare' repo that is operated on by git2
	// and try the 'pack-verify' with the less-time algorithm for reproduction.

With more context, this is:

gitoxide/git-pack/src/cache/delta/from_offsets.rs

Lines 100 to 110 in 8f9a55b

	RefDelta { base_id } => {
	// TODO: ref-deltas can also point to objects (by offset) which we haven't seen yet, so this should really be
	// handled maybe by delaying those? Add-Child needs things ordered ascending though, so it's not trivial to do
	// and probably requires tham offset-rewriting.
	// Note that some packs created by libgit2 generally look like that, see the 'index-bare' repo that is operated on by git2
	// and try the 'pack-verify' with the less-time algorithm for reproduction.
	resolve_in_pack_id(base_id.as_ref())
	.ok_or(Error::UnresolvedRefDelta { id: base_id })
	.and_then(|base_pack_offset| {
	tree.add_child(base_pack_offset, pack_offset, data).map_err(Into::into)
	})?;

But the code comment, which crate-status.md links to, went away in 84ade1d. The commit message there is:

fix: Allow resolution of in-pack ref-deltas (#287)

This finally allows delta tree caches to be used on typical small packs returned by GitHub.

So maybe the item in crate-status.md is (partially?) completed or otherwise should be revised?

In general-tasks.md

That link is a 404, both because it specifies gix-odb at a commit before git-odb was renamed to gix-odb, and because the structure and content of the crate has changed such that I am not sure if any corresponding code still exists:

gitoxide/general-tasks.md

Lines 15 to 19 in 795962b

	### gix-odb
	* Finding an object in a loose object store [costs an extra disk IO operation][extra-iop] to pacify the borrow checker. This wouldn't be an issue with polonius.
	[extra-iop]: https://github.com/GitoxideLabs/gitoxide/blob/2958145a0ae1ef582bbf88352f5567d5c2b5eaf0/gix-odb/src/store/linked/find.rs#L33

Changing the path back to git-odb gives:

gitoxide/git-odb/src/store/linked/find.rs

Line 33 in 2958145

if db.loose.contains(id) {

With more context, this is:

gitoxide/git-odb/src/store/linked/find.rs

Lines 11 to 14 in 2958145

	impl crate::Find for linked::Store {
	type Error = compound::find::Error;
	fn find<'a>(

gitoxide/git-odb/src/store/linked/find.rs

Line 22 in 2958145

match db.internal_find(id) {

gitoxide/git-odb/src/store/linked/find.rs

Lines 32 to 35 in 2958145

	None => {
	if db.loose.contains(id) {
	return db.loose.find(id, buffer).map_err(Into::into);
	}

As far as I can tell, this went away without being replaced by anything equivalent in 8c5ae77:

change!: Remove deprecated compound and linked object databases

The dynamic/general store is the only maintained can-do-it-all DB now.

So maybe this item should be removed (or heavily revised?) from general-tasks.md?

In gix-diff/src/tree/function.rs

The git_cmp_rs link in the gix_diff::tree::function::diff function's documentation comment is a 404, because it specifies a gix-object path at a commit before git-object was renamed to gix-object, and because the crate has been substantially reorganized:

gitoxide/gix-diff/src/tree/function.rs

Line 18 in 795962b

/// * Tree entries are expected to be ordered using [`tree-entry-comparison`][git_cmp_c] (the same [in Rust][git_cmp_rs])

gitoxide/gix-diff/src/tree/function.rs

Line 27 in 795962b

/// [git_cmp_rs]: https://github.com/GitoxideLabs/gitoxide/blob/a4d5f99c8dc99bf814790928a3bf9649cd99486b/gix-object/src/mutable/tree.rs#L52-L55

Changing the path back to git-object gives:

gitoxide/git-object/src/mutable/tree.rs

Lines 52 to 55 in a4d5f99

	fn cmp(&self, other: &Self) -> Ordering {
	let len = self.filename.len().min(other.filename.len());
	self.filename[..len].cmp(&other.filename[..len])
	}

With more context, this is:

gitoxide/git-object/src/mutable/tree.rs

Lines 50 to 56 in a4d5f99

	impl Ord for Entry {
	/// Entries compare by the common portion of the filename. This is critical for proper functioning of algorithms working on trees.
	fn cmp(&self, other: &Self) -> Ordering {
	let len = self.filename.len().min(other.filename.len());
	self.filename[..len].cmp(&other.filename[..len])
	}
	}

This changed substantially in ca37915 (#849). The comment went away, and the implementation was modified to fix #848. Although some code has moved around in the module, that implementation remains today:

gitoxide/gix-object/src/tree/mod.rs

Lines 263 to 273 in 795962b

	impl Ord for Entry {
	fn cmp(&self, b: &Self) -> Ordering {
	let a = self;
	let common = a.filename.len().min(b.filename.len());
	a.filename[..common].cmp(&b.filename[..common]).then_with(|| {
	let a = a.filename.get(common).or_else(|| a.mode.is_tree().then_some(&b'/'));
	let b = b.filename.get(common).or_else(|| b.mode.is_tree().then_some(&b'/'));
	a.cmp(&b)
	})
	}
	}

I'll open a PR that changes the git_cmp_rs link in the gix_diff::tree::function::diff documentation comment to point there. [Edit: I've opened #1628 for this.]

Expected behavior 🤔

I think references to code should either be current or otherwise that their significance to future development should be clear. For this reason, even though all links work when gix-* is changed back to git-*, I did not make any such changes in #1624 and I am not recommending them here, since it would then not be obvious how the text referencing the code ought to be understood.

Git behavior

Not applicable.

Steps to reproduce 🕹

See above.

[Byron]

Thanks a lot for collecting this information!

[about ref-deltas in crate-status] So maybe the item in crate-status.md is (partially?) completed or otherwise should be revised?

I am not aware of any shortcomings anymore regarding ref-deltas, so I think I can safely say that bullet-point can be removed.

So maybe this item should be removed (or heavily revised?) from general-tasks.md?

Yes, I think so. Initially there was an implementation that would try to be very smart, but since it was replaced by something much simpler.

Overall, I think that general-tasks.md is quite stale, even though I try to keep crate-status.md uptodate with the state each crate is in. I don't really have general-tasks.md on my radar, and it should probably be removed in favor of some adjustments to crate-status.md so there is one place for every potential task. This is not related to anything in this issue though, just a general remark.

[EliahKagan]

Thanks. Based on this, I've opened #1631 for the rest.

[EliahKagan]

How does the overall situation with general-tasks.md and crate-status.md relate to #470? Should anything be moved/copied to or from there as well?

[Byron]

Since general-tasks.md is already pretty much unmaintained, it's more like a failed experiment. From my experience I consult only crate-status.md, and usually it makes sense to split features by the crate that contains them.
This probably means that general-tasks.md can be dissolved, and frankly, probably nobody would miss it if it was just deleted.