feat: read global and system git config

[jayree]

I'm adding logic to read global and system git config:

• document the parameter in the JSDoc comment above the function
• add a test case in __tests__/test-X.js if possible
• squash merge the PR with commit message "feat(X): Added 'bar' parameter"

[jayree]

fixes: #236 #690

[dead-end]

Hi, I just looked at your code, especially the GitConfigManager. I am not sure that hard coding the config paths depending on the platform is the best approach. The GitConfigManager is called with a filesystem, which is an abstraction and can have different implementations. You can use for example a zip-backed readonly implementation for unit test (the ZipFS preserves timestamps, by the way). Depending on the platform where the node is running you search inside this filesystem for git configs with different paths. The config paths depend on the filesystem you are working with not on the platform where node is running. This can be the the same but it does not have to.

[jayree]

@dead-end I think figuring out what type of "fake" FS is isomorphic-git is running in is next to impossible. Everyone can come up with his own implementation (@isomorphic-git/lightning-fs itself is an example). Furthermore, the system and global config topic is about native OS use cases, with a native git installation. As far as I can tell, depending on the OS and binary, git has these paths hardcoded via the makefile as well. I'm also not a fan of hard coding the paths, a dependency on the child_process library to call git config --list --show-origin is even worse.

[dead-end]

@jayree I think this is reason why the issue was open for such a long time. By the way, the fix with the timestamps has a similar issue. The switch in the Filesystem.js assumes that filesystem implementation supports bigint if it is not running in the browser, which is not necessarily true.

      if (!process.browser) {
        const statsNs = await this._lstat(filename, { bigint: true })
        stats.mtimeNs = statsNs.mtimeNs

I think the biggest problem with your implementation is, that you cannot change the hard coded values. What we are interested in are some environment variables:

• HOME
• GIT_CONFIG_GLOBAL
• GIT_CONFIG_SYSTEM

Maybe it is a solution to pass an optional map of environment variables to the constructor of the filesystem wrapper and compute a hard code map from the platform as a default. Then you can use something like fs.env.HOME and fs.env.GIT_CONFIG_SYSTEM to search for the config files. If the env variables are not defined then you can assume that there are no config files.

You can provide three functions createWinEnv, createMacEnv, createUnixEnv that return hard coded maps and an other createPlatformEnv which detects the platform and then calls to appropriate function.

If a user uses ZipFS with a Mac style content he can use createMacEnv to create the filesystem.

[dead-end]

A simpler way is to use the optional env map for the filesystem constructor and expose the platform env as a fallback and simply use fs.env.HOME and fs.env.GIT_CONFIG_SYSTEM if they are defined with whatever value it has.
So a user who wants its global git config to be used has to set the env variables for his node. In this case we have only hard coded env variable names and no hard coded platform dependent paths.

I think that would be my perverted solution compared to the other.

[jcubic]

I also think that detecting the platform and hardcoding the paths is a wrong idea.

Please use what I suggested in #236 some kind of extension that allows you to set the path to global git config.

The current implementation of isomorphic-git doesn't have plugins anymore. So I think that the best would be to use the option:

home: '/home/jcubic/'

or maybe globalConfig: '/home/jcubic/.gitconfig' to every command. It may complicate things but the user code (suggested by docs) use object with a common settings:

const settings = {
   dir: '/home/jcubic/repo',
   globalConfig: '/home/jcubic/.gitconfig',
   url: '....'
}

await git.clone({...settings });
await fs.writeFile('/home/jcubic/repo/xxx', 'xxxx');
git.add({...settings, filepath: '...');

[jayree]

@jcubic While I understand that your proposal may serve its purpose in browser use cases where reliance on file system (FS) abstraction is necessary, I fail to see the advantages of referencing a "global" git configuration in a scenario where you have complete control over the FS content, abstraction and most importend no native git installation.

However, for CLI scenarios it is critical to depend on the system and global Git configuration and to follow the goal of achieving 100% interoperability with the canonical Git implementation in Isomorphic-Git.

Of cause, hardcoded file paths are bad, but these paths are hardcoded in the git binary as well. So the question is, do you want to support native Git Cli scenarios where isomorphic-git and native git coexist or not?

[jcubic]

I don't think that canonical git hardcoded the path on different systems. I think they use $HOME path or something similar to find the home directory. I would also prefer to have git config in the home directory that every user can edit.

[jayree]

@jcubic thats correct for the global config but the system config is hardcoded via makefile see: https://github.com/git/git/blob/94486b6763c29144c60932829a65fec0597e17b3/Makefile#L619

[jcubic]

But using sysconfdir which is probably added by automake or autoconf is different that hardcoding the full path in the source code. I think that for node environment variable will be much better than detecting the OS. Later someone will come that the library doesn't work on Android or iSO and new hardcoded values will need to be added. Adding an env variable will solve this issue, and also adding documentation on how to set those variables on different systems.

[jayree]

@jcubic @dead-end I give up