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

Conditional includes in .gitconfig not supported #12041

Open
ferenczy opened this issue Apr 22, 2021 · 12 comments
Open

Conditional includes in .gitconfig not supported #12041

ferenczy opened this issue Apr 22, 2021 · 12 comments
Assignees
Labels
bug Confirmed bugs or reports that are very likely to be bugs priority-3 Bugs that affect small number of users and/or relatively cosmetic in nature

Comments

@ferenczy
Copy link
Contributor

Describe the bug

The GitHub Desktop client doesn't support .gitconfig's Conditional includes.

I'm using a single machine for both work and private development. Each of those requires a distinct Git user (name and email). I don't want to configure the Git user for each and every repository individually (as I usually forget to do so and notice it when there're several commits under a wrong user already). That's why I'm using the Conditional includes. That allows me to have an individual configuration for a specific directory and all its subdirectories, for example:

  • ~/development/ for personal projects with a personal user name/email
  • ~/work/ for projects I'm working on in my job

My base ~/.gitconfig contains something like the following:

# this is a global configuration valid for the whole machine
[user]
    name = My Private Name With Nickname
    email = private@email

# the following section tells Git to use (actually merge to this config) the config file `~/.gitconfig-work` 
# for all repositories located in the directory `~/work/` and all its subdirectories
[includeIf "gitdir/i:~/work/**"]
    # include the content of `~/.gitconfig-work` here, if it contains any directives which have been already defined, it will overwrite them
    path = ~/.gitconfig-work

The content of ~/.gitconfig-work:

[user]
    name = My Work Name
    email = corporate@email

And that's it. All repositories under the directory ~/work/ will be using the configuration from the base ~/.gitconfig merged with ~/.gitconfig-work (which overwrites anything already defined in the base config). Repositories located anywhere outside of that directory will be using just the configuration from the base config.

And Git reflects that. When I execute git config user.email under the directory ~/work/ or its subdirectories, it prints the e-mail configured in ~/.gitconfig-work.

The problem is that GitHub Desktop is always showing the configuration from the base config. When I open the Repository menu > Repository settings... and select Git config, the option Use my global Git config is selected and the name and the email from the base config are shown there. And, of course, it's also used as a commit author.

Version & OS

GitHub 2.7.2
Windows 10 Pro 64-bit

Steps to reproduce the behavior

  1. Create two configuration files as described above, one included in the other one
  2. Create a new repository under the directory you have specified in the includeIf directive (gitdir:...)
  3. Check if the included config is correctly applied by executing git config user.email in your repository
  4. Check the Git config section in the Repository settings... dialog in GitHub Desktop

Expected behavior

GitHub Desktop behaves consistently with the official CLI Git client. Also, for example, SourceTree behaves correctly.

Actual behavior

The Conditional includes are ignored by GitHub Desktop.

@steveward
Copy link
Member

Thanks for the report @ferenczy. I did test out conditional includes last year (and it worked), but that was prior to us shipping various config improvements (#11595 and #11591). @sergiou87 should have some thoughts about this since he worked on that feature.

@sergiou87
Copy link
Member

Thank you for bringing this up @ferenczy! ❤️

I just checked and the problem seems to be we're running git config --global user.email, and when a specific scope (like --global) is given, git config won't look for for included files like the one you have unless we also specify --includes.

We'll look into this and let you know 😄

@sergiou87 sergiou87 added bug Confirmed bugs or reports that are very likely to be bugs priority-3 Bugs that affect small number of users and/or relatively cosmetic in nature labels Apr 23, 2021
@ferenczy
Copy link
Contributor Author

ferenczy commented Apr 27, 2021

Thanks for your replies guys!

@sergiou87 You're right, I have tested it on my machine with the same results. I was afraid that you may be using some Git library that doesn't support the conditional includes, so it may be complicated to fix, but this looks pretty easy to fix to me. Should I take a look and possibly create a PR? Or am I just too naive? 🙂

Also, those conditional includes are configured for a specific directory so it makes sense to me to read them with the repository configuration rather than the global configuration, i.e. using git config user.email in the repository directory. In such a case, the conditional includes are applied without any special option.

@sergiou87
Copy link
Member

I think it should be easy to do, yeah. Please go ahead and submit a PR if you feel comfortable to do so! I'll assign the issue to you 😄

Just FYI those values will be read-only, you won't be able to change them from the app, only see them when you open the Repository settings. We assume that if a user set up such an unusual config structure, they know what they're doing 😅

@HadesStyx

This comment was marked as off-topic.

@HadesStyx

This comment was marked as off-topic.

@ferenczy
Copy link
Contributor Author

ferenczy commented May 5, 2021

@sergiou87 OK, I'll take a look at it. Is it just about changing command-line options of the Git command the GitHub desktop is executing or should I expect more changes, please. I mean, is it possible it will have any consequences/side-effects?

@HadesStyx Were those questions addressed to me? If so, I don't understand what you're asking, I'm afraid. 🙂

@sergiou87
Copy link
Member

@ferenczy yeah, it should be just that… but as everything, it might have unexpected consequences or side-effects 😂  I think the idea would be trying to keep the changes to only affect the Repository Settings screen, if possible.

Also, don't worry about HadesStyx, those were just random comments. We get a bunch of those every day 😅

@Bro3Simon
Copy link

@sergiou87 @ferenczy , Hi guys any progress on this? I ran into this issue today as well. The display on GitHub Desktop doesn't truly represent the git configurations.

@steveward
Copy link
Member

From #15734:

The feature request

Here is an example of what I have set up. It may be atypical, but hopefully this is still a valid feature request:

When managing my dotfiles, I have a global .gitconfig that is common to all machines/environments. That .gitconfig includes the following:

[include]
  path = .gitconfig.local

In this way, the global settings for certain values can be set on a per-machine/per-environment basis. Github Desktop always updates ~/.gitconfig with [user] values without following the [include] to see where else it might be "included" (or already exist).

I found issue 12041, but it seems this is still broken.

See --[no-]includes for context.

Proposed solution

Support [include] such that the [user] value can be sourced from any/all included files.

In my example above, some useful config values to have in .gitconfig.local might be:

[user] 
  name = <value here>
  email = <value here>

[credential]
  helper = <platform-specific value>

[diff]
  guitool = <platform-specific tool>

Hopefully this further demonstrates the usefulness of this feature.

@driverkt
Copy link

driverkt commented Feb 9, 2023

From #15734:

Thanks for copying here. :)

@0xdevalias
Copy link

0xdevalias commented Dec 17, 2023

RE: #12041 (comment)

I think it should be easy to do, yeah. Please go ahead and submit a PR if you feel comfortable to do so! I'll assign the issue to you 😄

Just FYI those values will be read-only, you won't be able to change them from the app, only see them when you open the Repository settings. We assume that if a user set up such an unusual config structure, they know what they're doing 😅

RE: #12041 (comment)

OK, I'll take a look at it. Is it just about changing command-line options of the Git command the GitHub desktop is executing or should I expect more changes, please. I mean, is it possible it will have any consequences/side-effects?

RE: #12041 (comment)

yeah, it should be just that… but as everything, it might have unexpected consequences or side-effects 😂 I think the idea would be trying to keep the changes to only affect the Repository Settings screen, if possible.

From a quick search of the codebase for user.email, it looks like the way this config is read/updated inconsistently (maybe by design) across GitHub Desktop; with some places seeming to use getGlobalConfigValue('user.email') / setGlobalConfigValue('user.email', email); and other places using getConfigValue(repository, 'user.email', true) / etc

All of getConfigValue / getGlobalConfigValue / setConfigValue / setGlobalConfigValue seem to be defined in desktop/app/src/lib/git/config.ts:

https://github.com/desktop/desktop/blob/development/app/src/lib/git/config.ts#L5-L29

https://github.com/desktop/desktop/blob/development/app/src/lib/git/config.ts#L154-L175

There also seems to be a addGlobalConfigValue:

/** Set the global config value by name. */
export async function addGlobalConfigValue(
name: string,
value: string
): Promise<void> {
await git(
['config', '--global', '--add', name, value],
__dirname,
'addGlobalConfigValue'
)
}

And addGlobalConfigValueIfMissing:

/** Set the global config value by name. */
export async function addGlobalConfigValueIfMissing(
name: string,
value: string
): Promise<void> {
const { stdout, exitCode } = await git(
['config', '--global', '-z', '--get-all', name, value],
__dirname,
'addGlobalConfigValue',
{ successExitCodes: new Set([0, 1]) }
)
if (exitCode === 1 || !stdout.split('\0').includes(value)) {
await addGlobalConfigValue(name, value)
}
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed bugs or reports that are very likely to be bugs priority-3 Bugs that affect small number of users and/or relatively cosmetic in nature
Projects
Development

No branches or pull requests

7 participants