.unwrap() calls will be removed if we migrate RefTarget to new Conflict-based
type. Some of them were previously .unwrap_or_default(), but they would panic
anyway inside ref_target_from_proto().
is_none()/is_some() are wrapped as is_absent()/is_present() respectively.
If we see new RefTarget as a Conflict, an "absent" target isn't an emptyish
value like None, but an add of single [None]. It seemed a bit weird to call
such target is_none().
RefTarget type will be a wrapper around Conflict<Option<CommitId>>, and the
match arms would be more verbose. Fortunately, new code looks simpler since
we can merge non-conflicting cases.
It's named after Conflict::from_legacy_form(). If RefTarget is migrated to
new Conflict type, from_legacy_form([], [add]) will create a normal target,
and from_legacy_form([], []) will be equivalent to the current None target.
That's why this function isn't named as RefTarget::conflict().
If RefTarget is migrated to new Conflict type, this function will create
RefTarget(Conflict::resolved(Some(id))).
We still need some .unwrap() to insert Option<RefTarget> into map, but maps
will be changed to store new RefTarget type, and their mutation API will
guarantee that Conflict::resolved(None) is eliminated.
Summary: Pure QOL improvement, not that this was too awful as it stood. But this
ultimately makes all builds faster, effectively for free; on my Linux machine,
debug link time for jj-cli goes from 3s to 1s.
Signed-off-by: Austin Seipp <aseipp@pobox.com>
Change-Id: I5a0d516363635710733e8bee8f208050
Summary: This is needed to pick up a bugfix to the 'mold' package in nixpkgs,
used in a follow up diff.
Signed-off-by: Austin Seipp <aseipp@pobox.com>
Change-Id: I1128164faed93921f84c35189a012152
Perhaps, all view.get_<kind>(name) functions can return reference, but I'm
not willing to change the interface at this point. I'll revisit this after
migrating Option<RefTarget> to new Conflict-based type.
If we migrate RefTarget to new Conflict-based type, it won't store
Conflict<CommitId>, but Conflict<Option<CommitId>>. As the Option will
be internalized, new RefTarget type will also represent an absent target.
The 'target: Option<RefTarget>' argument will be replaced with new RefTarget
type.
I've also renamed the function for consistency with the following changes.
It would be surprising if set_local_branch(name, target) could remove the
branch. I feel the name set_local_branch_target() is less confusing.
Summary: The Determinate Systems Nix installer is fast and importantly includes
Flake support by default on new installs.
Magic Nix Cache will help ease the build time a little by making sure things
like downloads from crates.io remain cached between builds; in the future if we
ever need to add any custom builds to the Nix expression it will come really in
handy. But in my experience I don't think it should slow down much, and might
speed some things up a whole lot.
Signed-off-by: Austin Seipp <aseipp@pobox.com>
Change-Id: I56e1592cbaa6c7a43b5bd57972b98ada
Summary: While I was developing #1846, I found a case where `nix flake check`
succeeded but `nix build` on the jujutsu expression failed, which was confusing
to me. If it had been merged, it would have broke the ability to use Nix to
install things.
Let's try to attack this, and punch a parallel job into the build matrix;
hopefully running the two jobs concurrently can help catch this without
making build time much worse.
Signed-off-by: Austin Seipp <aseipp@pobox.com>
Change-Id: If2520d69492fa6abe499c1cb1d51d6e1
This might look more complicated than the original code, but it clarifies
that we always eliminate a (remove, add) pair. The behavior slightly changed
since an absent ref (i.e. None) in "removes" can now be paired with an "add"
even if "removes" contained other ids. Before, it was possible only when
removes.is_empty().
The order of conflicted ids slightly changed since Conflict::simplify()
tries to preserve diff pairs. It shouldn't matter so long as the result is
stable.
Summary: I need this for another change I'm playing around with, because the
newer nixpkgs snapshot has a newer version of a tool. Just do a quick update to
get it out of the way.
Maybe one day, Dependabot can help solve this issue...
Signed-off-by: Austin Seipp <aseipp@pobox.com>
Change-Id: I803f72da8666ed75018f51ebe511737d
Git CLI rejects it (though the data model would probably work fine with
"HEAD" branch.) This ensures that HEAD@git in JJ world is not an exported
branch named "HEAD".
Summary: Someone on Discord mentioned that while using branches, they could
switch between branches like `m1` and `main`, but not `HEAD@git`, even though it
was color coded the same and located right next to the other branch names in the
default `jj log` output.
This confused me too at first, until I realized `jj` was telling me that the
given commit was the `HEAD` reference in the colocated git repo. Let's just
color these references differently in the default schema, to try and hint that
these aren't the same. `bright green` is arbitrary, but helps stand out; and
`green` was only picked because both it and `git` start with "g".
GitHub Issue: #1843
Signed-off-by: Austin Seipp <aseipp@pobox.com>
Change-Id: I66d53ea2b9155db1fec6cae8ba8cbd0f
Summary: On x86_64-linux, these options reduce the total number of bytes in the
`jj` binary by 40 percent, from 16MiB to 9.8MiB, while having relatively minimal
impact on build time, going from 58s to 1m15s.
While `strip=debuginfo` is already the default, `codegen-units=1` is doing
all the work here, and seems to have a rather miraculous effect despite not
meaningfully hurting compile time too much for me at least. This actually
will probably improve performance in some cases too, but it's likely hard to
quantify.
Ultimately, with or without this change, the dominant entity taking up most
of the compile time for the whole project is `jj-lib` and `jj-cli` crates. A
`--timings` report with Cargo indicates almost all dependencies (on my machine)
compile within 30s whether or not this change is in play, so the remaining time
is all on us.
Various other changes, such as using `opt-level=z` or `lto=thin`, had no real
visible effect. `lto=fat` was pretty successful, reducing total bytes by over
50% (7.8MiB), but at a nearly 2x link-time cost.
It *might* be worth exploring if something like `lto=thin` could improve
performance since it didn't meaningfully impact compile time or size any
further, but that's TBD.
Note: these numbers were performed with a wiped `target/` directory after each
run, and so all dependencies were compiled from scratch, on my 12-core Ryzen
5600X.
Signed-off-by: Austin Seipp <aseipp@pobox.com>
Change-Id: Ioyoulrmtwxypsrwwwysyylvmqxzttkmn
Summary: Nix packages tend to come "full extra pack of batteries"-included, so
adding watchman support makes sense. I'll also want it for my own experiments.
Signed-off-by: Austin Seipp <aseipp@pobox.com>
Change-Id: Ia99757bf7a39814529525efe423d527b
This generates profiles in the Google Chrome JSON tracing format. They can be opened in Chrome's `chrome://tracing` page or in tools like https://ui.perfetto.dev. Enable by running e.g. `JJ_TRACE=1 cargo run -- status`.
Summary: When building in CI, we just build everything from scratch in the `dev`
profile, so just turn off debuginfo and incremental compilation data. These
aren't reused, but more importantly take up more space and CPU time to generate;
that's more expensive on slower machines and when using The Cloud(TM) to build.
Together, these take a fresh build (`rm -rf target/` ahead of time) from about
40s to 30s on my machine.
Signed-off-by: Austin Seipp <aseipp@pobox.com>
Change-Id: Id6e706634dbaf88f6ccd7f457409ad7c
This is (almost) a result of running
cargo +nightly clippy --workspace --all-targets --fix \
-- -A 'clippy::needless_raw_string_hashes'
with yesterday's nightly clippy.
https://github.com/mitsuhiko/insta/issues/389 causes numerous additional
`needless_raw_string_hashes` warnings, but it will hopefully be fixed soon.
For now, I recommend appending the second line to your invocations.
This will help to rewrite refs::merge_ref_targets() to leverage Conflict
type. I'm not pretty sure if we'll want to do that, but this change seems
also good for the existing code.
