mrc-ide / orderly2

https://mrc-ide.github.io/orderly2/
Other
7 stars 2 forks source link

Improve handling of empty or detached Git repositories. #148

Closed plietar closed 2 months ago

plietar commented 3 months ago

When orderly runs a report from a Git repository, it records information about the current state of the repository in the packet metadata. This includes the current commit hash and the name of the current branch.

It is possible for either of these to not exist, in which case we need to make sure orderly behaves gracefully. The commit hash will be missing on a brand new repository which hasn't had any commits yet (or equivalently, an existing repository where git checkout --orphan was used. The branch name would be missing if the repository is in a "detached HEAD" state, in other words the user checked out a specific commit hash instead of a named branch.

The first situation, of an empty repository, was not supported by orderly at all. It would throw an error and fail to run the report entirely. In the second case, when on a detached HEAD, the Git library we use reports the branch name as "HEAD", and that is what we were recording in the metadata. This is a bit misleading since it is not an actual branch name.

Both cases have been modified to work without errors and to omit the missing values from the metadata.

plietar commented 3 months ago

I'm kind of on the fence whether the fields in the JSON object should be omitted or set to null. I went with the latter for now, but I'm open to change that. Once we agree on what to do I will need to make a change to the upstream schema first before merging this PR.

richfitz commented 3 months ago

I'm kind of on the fence whether the fields in the JSON object should be omitted or set to null

Set to null I think. If we can detect the empty commit that would be worth capturing perhaps though?