Skip to content
This repository was archived by the owner on Apr 12, 2019. It is now read-only.

GetCommit() returns a ErrNotExist if short commit ID does not exists #113

Merged
merged 2 commits into from
Feb 3, 2019

Conversation

agrn
Copy link

@agrn agrn commented Apr 14, 2018

Currently, GetCommit() returns a generic error if a short commit ID
does not exists in a repository.

When a commit is not found by git-rev-parse, it returns an errors
which contains "fatal: ambiguous argument". GetCommit() now search if
the error contains this string, and, if it does, returns an
ErrNotExist.

The idea is to allow commits to be accessed from gitea with a short
commit ID. Without this change, it would return a 500 Internal Server
Error when a short ID does not exists in the repository.

Currently, GetCommit() returns a generic error if a short commit ID
does not exists in a repository.

When a commit is not found by git-rev-parse, it returns an errors
which contains "fatal: ambiguous argument". GetCommit() now search if
the error contains this string, and, if it does, returns an
ErrNotExist.

The idea is to allow commits to be accessed from gitea with a short
commit ID.  Without this change, it would return a 500 Internal Server
Error when a short ID does not exists in the repository.

Signed-off-by: Alban Gruin <alban@pa1ch.fr>
@thehowl
Copy link

thehowl commented Apr 19, 2018

should we not also check for unknown revision or path not in the working tree.? For instance, googling ambiguous argument I can find this, which does indicate that there are more errors covered by this message.

@agrn
Copy link
Author

agrn commented Apr 21, 2018

I changed the comparison to unknown revision or path as it appears only once in git’s source.

`fatal: ambiguous argument` can be the beginning of two errors in
git. This changes the comparison to something less ambiguous.

Signed-off-by: Alban Gruin <alban@pa1ch.fr>
@agrn agrn force-pushed the al/short-commit-error branch from 4ba06a0 to d08a180 Compare April 22, 2018 09:24
@HoffmannP
Copy link

HoffmannP commented Nov 11, 2018

Related to #122?

@lunny
Copy link
Member

lunny commented Nov 11, 2018

@HoffmannP maybe, but need some work to detect it's a short commit id or a full commit id or other refs.

@lafriks
Copy link
Member

lafriks commented Nov 11, 2018

I think this is not about short or long ID's but for detecting 404

@techknowlogick techknowlogick merged commit d04f81a into go-gitea:master Feb 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants