hosted-git-info icon indicating copy to clipboard operation
hosted-git-info copied to clipboard

BREAKING CHANGE: set default git ref to `HEAD`

Open darcyclarke opened this issue 3 years ago • 1 comments
trafficstars

Why/What

  • change all default references/fallbacks to HEAD (instead of the master branch)

Links & References

  • https://github.com/npm/cli/issues/4867

darcyclarke avatar May 07 '22 00:05 darcyclarke

One thing we should add to this is the ability to differentiate between directing to files vs directories. hosted-git-info currently only has the concept of a treepath for each host, but GitHub serves files and directories differently when using HEAD as the committish portion of the path:

This change works great if we only point to directories:

curl -sSL -D - https://github.com/npm/cli/tree/latest/lib | grep -e HTTP -e 'location: http'
HTTP/2 200

curl -sSL -D - https://github.com/npm/cli/tree/HEAD/lib | grep -e HTTP -e 'location: http'
HTTP/2 200

But if point to files and use treepath, then GitHub serves redirects to a specific commit which is not ideal:

curl -sSL -D - https://github.com/npm/cli/tree/latest/lib/npm.js | grep -e HTTP -e 'location: http'
HTTP/2 301
location: https://github.com/npm/cli/blob/latest/lib/npm.js
HTTP/2 200

curl -sSL -D - https://github.com/npm/cli/tree/HEAD/lib/npm.js | grep -e HTTP -e 'location: http'
HTTP/2 301
location: https://github.com/npm/cli/blob/f281ec8a1aec43439281a8fca4c255b0d94a0c94/lib/npm.js
HTTP/2 200

But if we use HEAD and a new internal configuration blobpath:

curl -sSL -D - https://github.com/npm/cli/blob/HEAD/lib/npm.js | grep -e HTTP -e 'location: http'
HTTP/2 200

What to do?

I think we should land another breaking change with this:

  • Create a new per-host config called blobpath. GitHub would use blob as the value. Research to be done for other hosts.
  • Change file() to always use blobpath instead of treepath (if relevant, eg GitHub uses the host raw.githubusercontent.com which has no treepath portion of the url)
  • Keep browse() to use treepath
  • Create a new method browseFile() that will use blobpath for browsing directly to a file

lukekarrys avatar Aug 08 '22 19:08 lukekarrys

@lukekarrys it should be noted that introducing a blob path does not eliminate redirects.

Example:

If the ref that is provided does not exist but did at some point (ex. was renamed - which is the case for most master branches) you are redirected to the default branch via 302; ex.

curl -sSL -D - https://github.com/npm/cli/blob/master/lib/npm.js | grep -e HTTP -e 'location: http'
HTTP/2 302
location: https://github.com/npm/cli/blob/latest/lib/npm.js
HTTP/2 200

ref. https://github.com/npm/cli/blob/master/lib/npm.js

Looking into GitLab, it seems as though they use a 302 to redirect as well; ex.

curl -sSL -D - https://gitlab.com/eBay/ebay/-/tree/HEAD/README.md | grep -e HTTP -e 'location: http'
HTTP/2 302
location: https://gitlab.com/eBay/ebay/-/blob/HEAD/README.md
HTTP/2 200

I personally don't think redirects are inherently bad here & introducing a new differentiator for file vs. directory links doesn't change if/when you might get redirected. That said, if you feel strongly it's a better user experience to introduce blobpath then we can gate the next major until that work is done as well.

darcyclarke avatar Aug 18 '22 13:08 darcyclarke

it should be noted that introducing a blob path does not eliminate redirects.

My goal was not to completely eliminate redirects, but to try and stop redirects that redirect to a different url such as https://github.com/npm/cli/tree/HEAD/lib/npm.js -> https://github.com/npm/cli/blob/f281ec8a1aec43439281a8fca4c255b0d94a0c94/lib/npm.js.

lukekarrys avatar Oct 12 '22 18:10 lukekarrys