Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: add doc comparing jj to Sapling (#1708) #1716

Merged
merged 1 commit into from
Jun 28, 2023
Merged

Conversation

martinvonz
Copy link
Owner

Checklist

If applicable:

  • I have updated CHANGELOG.md
  • I have updated the documentation (README.md, docs/, demos/)
  • I have updated the config schema (src/config-schema.json)
  • I have added tests to cover my changes

@martinvonz martinvonz force-pushed the push-xwytnzrsssol branch 4 times, most recently from 853ac3b to 4d8d65b Compare June 20, 2023 11:46
Copy link
Collaborator

@PhilipMetzger PhilipMetzger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but it would be valuable if a Meta employee also took a look at it.

docs/related-work.md Outdated Show resolved Hide resolved
@quark-zju
Copy link
Contributor

Mostly look good to me. There are some future plans that might count as noticeable differences but I think it's fine not documenting them until they are available:

  • Sapling might integrate with the virtual filesystem, EdenFS, and indirectly watchman and buck2 (i.e. buck2 can check file content hashes without materializing them on the filesystem). That might justify why it does not co-exist in a same git checkout (since git does not integrate with EdenFS).
  • The underlying storage is different. Sapling mainly uses IndexedLog (and its variants RotateLog, and ZStore) so .sl inode count is bounded. As a trade-off Sapling uses file locking and might not work well with NFS (although EdenFS technically can expose its Thrift RPC over the network for remote use-cases). It's a different trade-off from jj. Although right now it's mostly libgit2 and Sapling does not yet use its internal storage library yet.

Copy link

@zzl0 zzl0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM overall. Just two minor comments.

docs/sapling-comparison.md Show resolved Hide resolved
docs/sapling-comparison.md Outdated Show resolved Hide resolved
docs/sapling-comparison.md Outdated Show resolved Hide resolved
@martinvonz
Copy link
Owner Author

Mostly look good to me. There are some future plans that might count as noticeable differences but I think it's fine not documenting them until they are available:

Thanks for reviewing! Yes, I think it's better to document that once it's done. Feel free to send a PR when it is.

  • Sapling might integrate with the virtual filesystem, EdenFS, and indirectly watchman and buck2 (i.e. buck2 can check file content hashes without materializing them on the filesystem). That might justify why it does not co-exist in a same git checkout (since git does not integrate with EdenFS).

That actually sounds similar to what we're thinking of doing with jj at Google. I hope we'll be able to do that by replacing the working copy implementation (it's not currently a trait, but it will be soon). So we'll have the default working-copy implementation, which is used outside of Google, and we'll have a custom CitC-aware implementation internally. The right working-copy implementation will be loaded depending on the repo, just like we load the right commit backend etc. So maybe the main difference will be that your VFS-integration will be open-sourced.

  • The underlying storage is different. Sapling mainly uses IndexedLog (and its variants RotateLog, and ZStore) so .sl inode count is bounded. As a trade-off Sapling uses file locking and might not work well with NFS (although EdenFS technically can expose its Thrift RPC over the network for remote use-cases). It's a different trade-off from jj. Although right now it's mostly libgit2 and Sapling does not yet use its internal storage library yet.

Ah, that's a good point. I think it's probably too rare that users care about NFS support that it's worth mentioning.

@strager
Copy link

strager commented Jun 20, 2023

If jj supports Git tags, you could mention that Sapling does not support Git tags.

@martinvonz
Copy link
Owner Author

If jj supports Git tags, you could mention that Sapling does not support Git tags.

Thanks, but jj has very limited support (you can see and specify revisions by tag name, but you can't create tags), so I don't think it's worth mentioning.

@martinvonz martinvonz merged commit e6d5df1 into main Jun 28, 2023
14 checks passed
@martinvonz martinvonz deleted the push-xwytnzrsssol branch June 28, 2023 04:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants