Skip to content

cleanup and init a bare repository #185

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

Merged
merged 13 commits into from
Aug 30, 2021
Merged

cleanup and init a bare repository #185

merged 13 commits into from
Aug 30, 2021

Conversation

Byron
Copy link
Member

@Byron Byron commented Aug 29, 2021

This time we can still get around using a full-blown git-config implementation as well.

  • refactor and some tests
  • repository open()
  • use thiserror in favor of quickerror (thanks to transparent errors and it being used already in dependency tree)
  • init bare
  • get git-config parsing to work on windows

Comment on lines +86 to +88
let is_bare = config
.value::<Boolean<'_>>("core", None, "bare")
.map_or(false, |b| matches!(b, Boolean::True(_)));
Copy link
Member Author

Choose a reason for hiding this comment

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

@edward-shen I hope you are doing well!

Now that I am using git-config for the first time, I could imagine accessing values more comfortably. Unless I am missing something, are there plans to make this more ergonomic?
I could probably add what would help me with more convenient access, but love to align with you first. Thank you

Copy link
Contributor

Choose a reason for hiding this comment

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

That's the intent of ResolvedGitConfig. The current GitConfig was just a lightweight wrapper around the parser. I think I haven't written any accessor method on it, but that should be an easy add.

As for when I can get work done, I'm currently in the process of a cross continent move (NY -> CA), so unfortunately I don't know when I can work on it.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's the intent of ResolvedGitConfig. The current GitConfig was just a lightweight wrapper around the parser. I think I haven't written any accessor method on it, but that should be an easy add.

With this I am happy to wait, and if you have ideas on how this API should look like, I'd be happy to add these methods as I go.

@Byron
Copy link
Member Author

Byron commented Aug 30, 2021

@edward-shen It looks like on windows git-config can't parse a file that does parse fine on non-windows. It's based on this template and all I do is to substitute a value. I would appreciate any hints at what could cause this, and will look into it if that starts blocking me. Thanks a bunch.

Byron added 7 commits August 30, 2021 15:37
…which occour implicitly when checking out the repository fixtures on
windows, which are then windows formatted. From there the unholy
carriage return \r enters the compiled binary using include_bytes!
as a template, which then is used to initialize a repository.

In case of bare repositories, the implementation has to check if
it really is bare (i.e. bare = true) or just looks bare (because
there is no index, but that's the case in a new repository as well).

This check would use the config parser, which wouldn't consider carriage
returns and fail to parse the config file, making the opening of
repositories impossible on windows.
…previously each line would count doubly, now CRLF is counted as one
line, and NL is counted as one line too.
@Byron Byron merged commit dfbb015 into main Aug 30, 2021
@Byron Byron deleted the repository-init-bare branch August 30, 2021 14:05
@edward-shen
Copy link
Contributor

@edward-shen It looks like on windows git-config can't parse a file that does parse fine on non-windows. It's based on this template and all I do is to substitute a value. I would appreciate any hints at what could cause this, and will look into it if that starts blocking me. Thanks a bunch.

Maybe it's a newline issue? It's possible that the parser is always expecting a newline at that point, so it might not successfully parse it.

@edward-shen
Copy link
Contributor

edward-shen commented Aug 31, 2021 via email

@Byron
Copy link
Member Author

Byron commented Aug 31, 2021

Maybe it's a newline issue? It's possible that the parser is always expecting a newline at that point, so it might not successfully parse it.

I took a look and the first version of the fix was quickly supplanted by the second version which also counts newlines correctly again (I think).

I think something similar is fine,
e.g. a .value() method thats generic over some bytes form (TryFrom<[u8]>
was what I had before, but having a Deserialize bound might be nice), as
well as helper methods such as .bool() and .str() that make it easier to
access.

I will see how these utility methods could look like, the first stab at it would certainly be guided by my recent use of reading a boolean.

I don't have a strong vision on what the API should be like, and since
gitoxide is the primary user I see it perfectly reasonable to have one that
is best ergonomic for you.

Perfect, thank you! I will be pinging you solely for the purpose of keeping you in the loop and to encourage discussion, but know well enough that you are very busy. Lastly, please feel free to guide this communication to work for you, I wouldn't want that to be a burden.

Long story short, I will be very happy to get into git-config more, and hope you will have a good time in the new location.

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.

2 participants