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

Document Nixpkgs "installation method" #11

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mightyiam
Copy link

It's not yet in there...

But I hope it will be soon.

And thanks for this useful app!

@quodlibetor
Copy link
Owner

Neat! I'll merge this as soon as that PR is accepted. Thanks!

@quodlibetor
Copy link
Owner

please ping this pr again then that one is merged, I probably won't be tracking it closely.

@mightyiam
Copy link
Author

Hey, I got this comment there I don't particularly agree with, but I'm also not one to argue over such things. Would you have an idea how you'd have this named under the Nixkpgs namespace? quodlibetor-git-fixup, perhaps? Note that the executable name is not in question—that will remain git-fixup.

@quodlibetor
Copy link
Owner

ehh I've considered renaming this to git-instafix because there are a bunch of git-fixup projects out there, you can even see that in the name of some env vars.

I can create a new project after the break that has that name, or you can feel free to propose it as an alternative name.

The big difference between this and the git-fixup that that maintainer linked to is that this project provides a richer ui for selecting the fixup commit, and automatically rebases for you.

@mightyiam
Copy link
Author

Used a synonym list and a Google search to come up with these:

name status
git-fixdown taken
git-alter free
git-rectify free
git-revise taken
git-revamp free
git-rework free
git-re free

My favorite is git-re

@mightyiam
Copy link
Author

Need help deciding?

@quodlibetor
Copy link
Owner

thanks for the synonym list! I had a flu over the holidays and am still recovering, I'd hoped to get things cleaned up this last weekend but obviously didn't.

I'll see how this week goes, I'm excited that you're interested and will try and get it out but I've got a lot of life right now.

@mightyiam
Copy link
Author

I wish you a graceful recovery, Brandon. I changed my mind. My favorite is your git-instafix. It's amusing and catchy.

@mightyiam
Copy link
Author

How are you, Brandon?

@quodlibetor
Copy link
Owner

I've updated the project to git-instafix and pointed out similar projects.

Thanks for your patience!

@mightyiam
Copy link
Author

Thank you, @quodlibetor . I've updated the nixpkgs PR.

@kuruczgy
Copy link

Note: the nixpkgs package would be more elegant if the path git binary were configurable, maybe consider adding this option in the future if you have time. (But you state that you intend to remove all dependencies on the git binary anyway, so this issue might just resolve itself.)

@quodlibetor
Copy link
Owner

But you state that you intend to remove all dependencies on the git binary anyway

yeah currently the only dependency is for rebase, which gitoxide does not support and git2 didn't support. It's worth revisiting now that someone managed to get it merged into git2.

@quodlibetor
Copy link
Owner

the nixpkgs package would be more elegant if the path git binary were configurable

Do you mean with a config file? Right now it should just pull it from PATH.

@kuruczgy
Copy link

Do you mean with a config file? Right now it should just pull it from PATH.

With a compile time config. (I am sure there is some way to do this with cargo...) On NixOS programs can't rely on anything being in PATH, all dependencies have to be declared explicitly. If you look at NixOS/nixpkgs#275402, right now an extra wrapper has to be defined just to provide a PATH value with the git binary.

@mightyiam mightyiam marked this pull request as draft February 26, 2024 09:39
@quodlibetor
Copy link
Owner

With a compile time config.

Yeah that's easy enough to do, and there's a few ways. Do you have an example of something in any language being built in a way that follows nix best practices?

@kuruczgy
Copy link

I looked around a bit, it seems that compile time env variables are the most common way to do this. Examples here and here.

You can use the option_env! macro to get an env variable at compile time. Then the logic would be to let's say use std::option_env!("GIT_BINARY_PATH"), if it returns a Some then use it, otherwise default to "git" as you currently do.

@quodlibetor
Copy link
Owner

perfect, thanks.

quodlibetor added a commit that referenced this pull request Mar 4, 2024
There was only one remaining use of the external git dependency in
git-instafix, and it was just to print diffs.

Replacing that with our home-grown git diff code is currently a slightly
worse experience (it doesn't respect user's git diff config) but it is
good enough, and it sets this up for a long term of having significantly
more control.

This also resolves a slight annoyance in the nix package (from [this
comment](#11 (comment)))
@quodlibetor
Copy link
Owner

Version 0.2.1 (just released) switched the last piece from an external git command to libgit2, it no longer has any dependency on the git binary.

@mightyiam
Copy link
Author

W00t

@mightyiam
Copy link
Author

Updated the PR.

README.md Outdated
@@ -55,6 +55,8 @@ You can also install from this repo with `cargo`:

cargo install --git https://github.com/quodlibetor/git-fixup

Or use Nixpkgs. It's [in there](https://search.nixos.org/packages?channel=unstable&query=git-fixup).
Copy link
Owner

Choose a reason for hiding this comment

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

Could you include the command people would run? Something like this:

Or use [Nixpkgs](https://search.nixos.org/packages?channel=unstable&query=git-fixup):

	<the exact thing users would type>

Copy link
Author

@mightyiam mightyiam Mar 16, 2024

Choose a reason for hiding this comment

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

In Nix a package can be obtained in several different ways. I feel it's appropriate to not provide any examples in contexts such as this.

@mightyiam mightyiam marked this pull request as ready for review March 16, 2024 06:39
@quodlibetor
Copy link
Owner

Sorry about the delay on this, I don't understand nix enough to know how I want to document it. I was trying to teach it but I'll just ask you.

My big concerns/questions:

  • How does the nix package get updated? Is it automatic, and if so are there docs?
  • does this being in nix mean it works with devbox, flakes, or other similar wrappers?
  • what would be the easiest way for me to test the nix package?

@mightyiam
Copy link
Author

mightyiam commented Jul 1, 2024

Sorry about the delay on this, I don't understand nix enough to know how I want to document it. I was trying to teach it but I'll just ask you.

Sure. Look, Nix is a lot to learn. And there are different ways of using it. Therefore, there are no good answers:

  • How does the nix package get updated? Is it automatic, and if so are there docs?

Depends on how it is used.

  • does this being in nix mean it works with devbox, flakes, or other similar wrappers?

I'm not familiar with devbox. nixpkgs offers a flake. That flake includes git-instafix.

  • what would be the easiest way for me to test the nix package?

Have nix with flakes enabled (install it via the DetSys installer) and run:

nix run nixpkgs#git-instafix -- --version

Learn more about nix at https://nix.dev . Feel free to message me for help, as well.

@quodlibetor
Copy link
Owner

Oh when I said this:

How does the nix package get updated? Is it automatic, and if so are there docs?

I meant how does nixpkgs itself get the git-instafix update.

Looking at this PR it looks like it's automated someway? NixOS/nixpkgs#322675 ? Is there anything I need do to make sure that it continues to work correctly? I mostly want to make sure that if something has a documented install method I know how to check if it's up to day and what went wrong if it's not up to date.

@mightyiam
Copy link
Author

I meant how does nixpkgs itself get the git-instafix update.

Looking at this PR it looks like it's automated someway? NixOS/nixpkgs#322675 ? Is there anything I need do to make sure that it continues to work correctly? I mostly want to make sure that if something has a documented install method I know how to check if it's up to day and what went wrong if it's not up to date.

You're right. That is automated. But I don't know how it works. Probably looks at git tags in your repo.

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.

None yet

3 participants