nips icon indicating copy to clipboard operation
nips copied to clipboard

nip34: add refs to git repo event

Open DanConwayDev opened this issue 1 year ago • 14 comments

so that the event can be used as a source of truth for the state of refs such as branches and tags.

this could be useful as:

  1. a way to reduce trust in git server.(s) so they no longer act as a source of truth. A nip34 git remote-helper could proxy requests to git servers and only pull updates when they match the state listed in the repo event.
  2. a form of authorisation for nip34 git server implementations such as song
  3. a way of enabling experimentation with other protocols for hosting and accessing git data. eg a blossom git remote helper

DanConwayDev avatar May 21 '24 14:05 DanConwayDev

Are you doing another array inside the tag array?

["refs", ["heads/<branch-name>","<commit-id>"], ...]

Is that possible? Tags are supposed to be strings not arrays.

Why not just multiple refs?

["refs", "heads/<branch-name>","<commit-id>"],
["refs", "heads/<branch-name>","<commit-id>"],

vitorpamplona avatar May 21 '24 15:05 vitorpamplona

I like this, but I don't know if it's better than having a different kind like

{
  "kind": 1034,
  "tags": [
    ["r", "heads/master", "<commit-id>"]
  ]
}

and then we can have a bunch of these? But relays should probably delete old ones.

I guess the replaceable thing works better.

But then maybe we should have a different kind so the core repository metadata doesn't have to be updated every time we push a commit:

{
  "kind": 30618,
  "tags": [
    ["d", "<repo-id>"],
    ["refs", "<heads|tags>/<branch-or-tag-name>","<commit-id>"] // can appear multiple times for multiple refs
  ]
}

fiatjaf avatar May 21 '24 15:05 fiatjaf

@vitorpamplona, your right, I updated it to reflect your suggestion.

Less updates to the announcement event would be preferable. There may be a scenario where the author wasn't to stop tracking state using nostr but later changes there mind.

I've tried to address that with this update.

DanConwayDev avatar May 21 '24 16:05 DanConwayDev

I previously wrote some code that listed a number of <parent-commit-id> as additional values in the refs tag.

This improves the UX when the state event is ahead of the git server. Currently in this scenario git pull wouldn't know how far ahead / behind the state event is from the git server. Listing the parent ids would enable reporting such as 3 ahead 2 behind.

This could be added as an optional part of the nip. What do you think?

DanConwayDev avatar May 21 '24 16:05 DanConwayDev

If there can be multiple maintainers who are able to push the master branch, we need a mechanism to find out which maintainer has pushed the last time. We should fetch all kind 30618 events, and use the latest one as source of truth about the ref "heads/master" points to.

lez avatar May 21 '24 17:05 lez

Why not just multiple refs?

["refs", "heads/<branch-name>","<commit-id>"],
["refs", "heads/<branch-name>","<commit-id>"],

Maybe we are better off staying closer to git internal representation of refs. This one is easier to deal with in code, and it's easier to understand for devs who are already familiar with git.

["refs/heads/master", "<commit-id>"],
["refs/tags/v1.0.0", "<commit-id>"],
["HEAD", "ref: refs/heads/master"],

lez avatar May 24 '24 01:05 lez

If there can be multiple maintainers who are able to push the master branch, we need a mechanism to find out which maintainer has pushed the last time. We should fetch all kind 30618 events, and use the latest one as source of truth about the ref "heads/master" points to.

Many people can publish 30618 events pointing to the same git server, if that's what you mean. Should work fine, you can pick whatever you want -- although I think that users should get just the last one issued by the pubkey that they typed by default.

fiatjaf avatar May 24 '24 03:05 fiatjaf

Maybe we are better off staying closer to git internal representation of refs. This one is easier to deal with in code, and it's easier to understand for devs who are already familiar with git.

["refs/heads/master", "<commit-id>"],
["refs/tags/v1.0.0", "<commit-id>"],
["HEAD", "ref: refs/heads/master"],

Either way is fine to me. This looks more elegant, but the other is easier to write simple functions that parse and list all refs.

fiatjaf avatar May 24 '24 03:05 fiatjaf

listing additional maintainers in the announcement event signals trust in the announcement and state events they publish against the same identifier.

clients should fetch events published by all of these pubkeys and use the one with the largest non-future created_at parameter.

I touched on the reason in the original nip34 PR:

Should I have to wait for the specific maintainer that I am referencing to update their repo event before I can pull the latest changes that another maintainer pushed a day or so ago? This is not a good UX and user may bypass the nostr event and trust the pgp signed commits on the git server.

https://github.com/nostr-protocol/nips/pull/997#issuecomment-1908038483

DanConwayDev avatar May 24 '24 06:05 DanConwayDev

["refs/heads/master", "<commit-id>"],
["refs/tags/v1.0.0", "<commit-id>"],
["HEAD", "ref: refs/heads/master"],

Either way is fine to me. This looks more elegant, but the other is easier to write simple functions that parse and list all refs.

I think lez is right. staying closer to git internals is probably better. Clients would want to selectively filter the tags anyway to avoid using refs/remote/*, etc.

DanConwayDev avatar May 24 '24 06:05 DanConwayDev

["refs/heads/master", "<commit-id>"],
["refs/tags/v1.0.0", "<commit-id>"],
["HEAD", "ref: refs/heads/master"],

Either way is fine to me. This looks more elegant, but the other is easier to write simple functions that parse and list all refs.

I think lez is right. staying closer to git internals is probably better. Clients would want to selectively filter the tags anyway to avoid using refs/remote/*, etc.

We could get the best of both approaches by using

["ref", "refs/heads/master", "<commit-id>"],
["ref", "refs/tags/RELEASE_v1.0", "<commit-id>"],
["HEAD", "ref: refs/heads/master"]

lez avatar May 26 '24 18:05 lez

Is this ready to merge?

fiatjaf avatar Jun 06 '24 18:06 fiatjaf

I just made the suggested changes. Don't we need some production implementations to merge this?

DanConwayDev avatar Jun 07 '24 10:06 DanConwayDev

I just made the suggested changes. Don't we need some production implementations to merge this?

the implementation I'm aware of (far from production): https://github.com/lez/git-remote-nostr

lez avatar Jun 09 '24 18:06 lez

I've implemented this in a git remote helper.

The vision here is for git servers to act just as dumb data relays.

I plan to release it once, I've added some features around pulling and pushing nip34 patches.

Feel free to try it out by building from the main branch.

---more details--- It uses the state announcement as the source of truth and downloads commits from one of the git servers listed in the clone tag of the repo announcement.

If a state announcement doesn't exist it falls back to proxying the first git server listed the repo announcement.

If it successfully pushes to one of the git server, it then updates the state announcement, before attempting to push to the remain git severs.

@fiatjaf let me know if you plan to implement this as an alternative authorization for song and I'll get it to broadcast the event first.

DanConwayDev avatar Aug 01 '24 10:08 DanConwayDev

I plan many things, but I haven't touched the git stuff in a while. I have to understand it better before implementing but these sound like good ideas. We should just merge this if you tell me it's still up-to-date.

fiatjaf avatar Aug 01 '24 16:08 fiatjaf

I plan many things, but I haven't touched the git stuff in a while. I have to understand it better before implementing but these sound like good ideas. We should just merge this if you tell me it's still up-to-date.

I implemented it as written and it seems to work alright.

DanConwayDev avatar Aug 01 '24 16:08 DanConwayDev