Skip to content
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

git-config towards 1.0 #331

Closed
24 tasks done
Byron opened this issue Feb 8, 2022 · 8 comments
Closed
24 tasks done

git-config towards 1.0 #331

Byron opened this issue Feb 8, 2022 · 8 comments
Labels
C-tracking-issue An issue to track to track the progress of multiple PRs or issues

Comments

@Byron
Copy link
Member

Byron commented Feb 8, 2022

An issue to see what's missing to make git-config comfortably usable by git-repository.

Tasks

  • ❗️a way to know about the source of a value, i.e. it's possible to know which config file is providing an obtained value.
    • this is important for reading values with security implications, so one would avoid accepting values from configuration files not owned by the current user.
    • a way to restrict queries to a sub-set of the available sources
    • git-config: secure access to paths and other values #460
      anymore as query entails git-sec.
  • git-config: reduced parsing mode #459
  • cascading config files (review/validate)
    • global config instantiation (the one without a local repository)
    • know which config files to cascade
  • handle implicit keys like k and k= losslessly during serialization
  • git-config output to std::io::Write #456
  • correct parsing of Colors (e.g. hex input, attributes, inverted attributes, order independence) - validate
  • use bstr internally instead of [u8]
  • flock config files before reading them
    • a linux-specific thread-safety mechanism, with nothing to do with flock.
  • advanced features
  • comfortable API for accessing values
  • validate sub-sections are case-sensitive when reading (and writing)
    • This applies only to legacy sub-section syntax and we don't implement that, comparing case-insensitive instead. Listed in known deviations.
  • remove ResolvedConfig entirely - GitConfig can do it all
  • a minimal gix repo config to print all values, helpful to test errors
  • review entire API via cargo doc

Related Discussions

Towards 2.0

  • full serde serialization support (maybe) - the current one is only partial.
  • resolved config (optimized for reading, no round-tripping) is created directly from parse events (like git does)
  • 100% correct date parsing with git-date
    • This includes parsing expiry dates, probably as its own top-level type in comfort.rs.

Other Ramblings

  • Could git-config use self-referential structures like in ignore to avoid copying everything into Cow<'static, [u8]> effectively.
@Byron Byron added the C-tracking-issue An issue to track to track the progress of multiple PRs or issues label Feb 8, 2022
Byron pushed a commit to svetli-n/gitoxide that referenced this issue Feb 15, 2022
…eLabs#331)

Add a `Path` type to the `git_config::values` which
can be interpolated according to gits own path interpolation
rules.
Byron added a commit to svetli-n/gitoxide that referenced this issue Feb 20, 2022
…bs#331)

The lack of context would make %(prefix)/… paths generally fail
which makes the API prone to misuse.

This probably means that another API needs to be added to facilitate
obtaining paths while providing a way to pass the context, too.

For now having the `Path` type as intermediary seems sufficient,
but also might point towards other a more approach.
Byron added a commit to svetli-n/gitoxide that referenced this issue Feb 20, 2022
This composes better with other errors, if necessary.
Byron added a commit to svetli-n/gitoxide that referenced this issue Feb 20, 2022
…f` (GitoxideLabs#331)

That way users will want to call it or something like it in order to
make use of the `Path`, which otherwise is just a bunch of bytes which
aren't very useful at all in standard Rust.

Keeping the `Path` wrapper type is probably the right choice as it
matches the current API and allows to get paths explicitly which can
then be interpolated in just another step.
Byron added a commit to svetli-n/gitoxide that referenced this issue Feb 20, 2022
Always a good idea to run all tests beforehand.
Byron added a commit to svetli-n/gitoxide that referenced this issue Feb 20, 2022
Always a good idea to run all tests beforehand.
Byron added a commit to svetli-n/gitoxide that referenced this issue Feb 20, 2022
Always a good idea to run all tests beforehand.
Byron added a commit to svetli-n/gitoxide that referenced this issue Feb 20, 2022
Always a good idea to run all tests beforehand.
Byron added a commit to svetli-n/gitoxide that referenced this issue Feb 21, 2022
This means we don't necessarily enforce slashes everywhere.

Intiially the implementation was meant to be as close to `git` as
possible, even though certain requirements there stem from the
assumption that the path separtor is `/`.

In Rust, however, we now return `std::path::Path` which deals with
that for us and allows to operate on paths more comfortably and safely.

We now leverage this fully even if that means that the paths we output
aren't entirely conformant with what git would use, but it's an
implementation detail.

Lastly, the interpolated path is a transformation step so there should
be no easy way to accidentally put that version of the path back
into the file, which would be undesirable due to the various
transformations.
Byron added a commit to svetli-n/gitoxide that referenced this issue Feb 21, 2022
)

Previously PathError was meant to be used primarily in `Path::try_from()`
so a case could be made to give it that name. Now the error is exclusive
to `Path`, so that should be reflected in the module location as well.

This helps with organizing the code as well, putting everything related
to Path into its own module, which helps refactoring later.
Byron added a commit to svetli-n/gitoxide that referenced this issue Feb 21, 2022
Byron pushed a commit that referenced this issue Feb 21, 2022
Add a `Path` type to the `git_config::values` which
can be interpolated according to gits own path interpolation
rules.
Byron added a commit that referenced this issue Feb 21, 2022
The lack of context would make %(prefix)/… paths generally fail
which makes the API prone to misuse.

This probably means that another API needs to be added to facilitate
obtaining paths while providing a way to pass the context, too.

For now having the `Path` type as intermediary seems sufficient,
but also might point towards other a more approach.
Byron added a commit that referenced this issue Feb 21, 2022
This composes better with other errors, if necessary.
Byron added a commit that referenced this issue Feb 21, 2022
…f` (#331)

That way users will want to call it or something like it in order to
make use of the `Path`, which otherwise is just a bunch of bytes which
aren't very useful at all in standard Rust.

Keeping the `Path` wrapper type is probably the right choice as it
matches the current API and allows to get paths explicitly which can
then be interpolated in just another step.
Byron added a commit that referenced this issue Feb 21, 2022
Always a good idea to run all tests beforehand.
Byron added a commit that referenced this issue Feb 21, 2022
This means we don't necessarily enforce slashes everywhere.

Intiially the implementation was meant to be as close to `git` as
possible, even though certain requirements there stem from the
assumption that the path separtor is `/`.

In Rust, however, we now return `std::path::Path` which deals with
that for us and allows to operate on paths more comfortably and safely.

We now leverage this fully even if that means that the paths we output
aren't entirely conformant with what git would use, but it's an
implementation detail.

Lastly, the interpolated path is a transformation step so there should
be no easy way to accidentally put that version of the path back
into the file, which would be undesirable due to the various
transformations.
Byron added a commit that referenced this issue Feb 21, 2022
Previously PathError was meant to be used primarily in `Path::try_from()`
so a case could be made to give it that name. Now the error is exclusive
to `Path`, so that should be reflected in the module location as well.

This helps with organizing the code as well, putting everything related
to Path into its own module, which helps refactoring later.
Byron added a commit that referenced this issue Feb 21, 2022
@Byron Byron linked a pull request Feb 26, 2022 that will close this issue
@Byron Byron removed a link to a pull request Feb 26, 2022
Byron added a commit that referenced this issue Mar 6, 2022
Byron added a commit to svetli-n/gitoxide that referenced this issue Mar 17, 2022
Byron added a commit that referenced this issue Jul 21, 2022
Byron added a commit that referenced this issue Jul 22, 2022
Byron added a commit that referenced this issue Jul 22, 2022
Byron added a commit that referenced this issue Jul 22, 2022
Byron added a commit that referenced this issue Jul 22, 2022
Byron added a commit that referenced this issue Jul 22, 2022
Byron added a commit that referenced this issue Jul 22, 2022
Byron added a commit that referenced this issue Jul 22, 2022
Byron added a commit that referenced this issue Jul 22, 2022
Byron added a commit that referenced this issue Jul 22, 2022
Byron added a commit that referenced this issue Jul 22, 2022
Byron added a commit that referenced this issue Jul 22, 2022
Byron added a commit that referenced this issue Jul 22, 2022
Byron added a commit that referenced this issue Jul 22, 2022
Byron added a commit that referenced this issue Jul 22, 2022
…onsiders. (#331)

Filters allow to narrow down the output.
@Byron Byron closed this as completed Jul 22, 2022
Byron added a commit that referenced this issue Jul 22, 2022
That way, applications which want to display or work with configuration
files can do so.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-tracking-issue An issue to track to track the progress of multiple PRs or issues
Projects
None yet
Development

No branches or pull requests

4 participants