Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Read and write git configuration without repository #2564

Merged
merged 17 commits into from
Nov 20, 2020

Conversation

smashwilson
Copy link
Contributor

@smashwilson smashwilson commented Nov 16, 2020

Reading and writing global or system git configuration entries, like user.email and user.name, makes sense even if the current repository is absent or uninitialized. Let's let these calls succeed by using a git strategy initialized with a workdir of the drive root.

This addresses #2558 by behaving correctly in a variety of situations where we should be able to read your identity from the global git configuration, but weren't. We prevent git commands from executing in contexts where we don't have a valid working directory to run them with - if a project root is not an initialized git repository, or if we have no project root yet, or if a git repository is still loading - but git config doesn't have this constraint; unless you specify --local, it's perfectly reasonable to read and write global or system git config values without a working directory. Allowing this to happen closes the windows of time during which the git tab can attempt to read user.name and user.email, get empty strings back from the stubbed getConfig methods on those repository states, and conclude that it needs to display the identity panel to prompt you for them.

I also needed to implement cross-instance cache invalidation among repositories in the WorkdirContextPool, so that setting your identity in one context would properly cause any other repositories in other contexts to pick up the changes that you wrote (and not, for example, immediately delete them).

Note that non-present repository states do not cache these config values, so they could be redundantly read frequently. I think that's okay for now because config reads are very quick. I also haven't implemented a filesystem watcher on the global configuration file to pick up external identity changes automatically.

@codecov
Copy link

codecov bot commented Nov 16, 2020

Codecov Report

Merging #2564 (3297581) into master (c47ddbd) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2564      +/-   ##
==========================================
+ Coverage   93.43%   93.44%   +0.01%     
==========================================
  Files         236      237       +1     
  Lines       13208    13232      +24     
  Branches     1900     1905       +5     
==========================================
+ Hits        12341    12365      +24     
  Misses        867      867              
Impacted Files Coverage Δ
lib/controllers/git-tab-controller.js 86.33% <100.00%> (+0.61%) ⬆️
lib/git-shell-out-strategy.js 99.81% <100.00%> (ø)
lib/models/repository-states/cache/keys.js 100.00% <100.00%> (ø)
lib/models/repository-states/present.js 100.00% <100.00%> (ø)
lib/models/repository-states/state.js 99.05% <100.00%> (+0.04%) ⬆️
lib/models/repository.js 96.21% <100.00%> (+0.05%) ⬆️
lib/models/workdir-context-pool.js 95.29% <100.00%> (+0.23%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c47ddbd...f466251. Read the comment docs.

@smashwilson smashwilson marked this pull request as ready for review November 20, 2020 20:46
@smashwilson smashwilson merged commit 2c9ffe3 into master Nov 20, 2020
@smashwilson smashwilson deleted the read-config-without-repo branch November 20, 2020 20:47
This was referenced Nov 27, 2020
@dnorthup-ums
Copy link

@smashwilson I'm not seeing the v0.36.6 update available in Atom yet, and I just upgraded it to 1.54.0 to make sure that wasn't the problem. Did you remember to push the release up to the Atom Package Manager server?

@smashwilson
Copy link
Contributor Author

I didn't forget... I hit a mysterious crash trying to do the actual upgrade in atom/atom#21782 that I haven't been able to get to the bottom of yet 😞 . I'm guessing that it'll take me two or three days of dedicated time on the Windows box, which I have not been able to muster.

The good news is that there's a partial fix in Atom 1.54.0 that should improve things somewhat in the meantime.

@nhassan201920
Copy link

#2643 to

Copy link

@Mirwais-Hakimy Mirwais-Hakimy left a comment

Choose a reason for hiding this comment

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

Hello world @ #

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants