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
fix: preserve merge states in submodules #769
Merged
Merged
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
1c436b9
fix: preserve merge states in submodules
iiroj d701f73
test: set git user details into submodule dir for Windows
iiroj 37ed3d1
fix: resolve absolute git config dir in submodule case
iiroj 10d6a11
fix: use String.prototype.trim() to remove whitespace from shell output
iiroj File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
'use strict' | ||
|
||
const normalize = require('normalize-path') | ||
const path = require('path') | ||
|
||
const execGit = require('./execGit') | ||
const { readBufferFromFile } = require('./file') | ||
|
||
/** | ||
* Resolve path to the .git directory, with special handling for | ||
* submodules | ||
*/ | ||
const resolveGitConfigDir = async ({ gitDir, isSubmodule }) => { | ||
const defaultDir = path.resolve(gitDir, '.git') | ||
if (!isSubmodule) return normalize(defaultDir) | ||
|
||
const buffer = await readBufferFromFile(defaultDir) | ||
const dotGit = buffer.toString() | ||
const gitConfigDir = path.resolve(dotGit.replace(/^gitdir: /, '')) | ||
return normalize(gitConfigDir) | ||
} | ||
|
||
/** | ||
* Resolve git directory and possible submodule paths | ||
*/ | ||
module.exports = async function resolveGitRepo(options = {}) { | ||
try { | ||
// git cli uses GIT_DIR to fast track its response however it might be set to a different path | ||
// depending on where the caller initiated this from, hence clear GIT_DIR | ||
delete process.env.GIT_DIR | ||
|
||
const gitDir = await execGit(['rev-parse', '--show-toplevel'], options) | ||
const superprojectWorkingTree = await execGit( | ||
['rev-parse', '--show-superproject-working-tree'], | ||
options | ||
) | ||
// A super-project working tree exists only in the context of submodules | ||
const isSubmodule = !!superprojectWorkingTree | ||
const gitConfigDir = await resolveGitConfigDir({ gitDir, isSubmodule }) | ||
|
||
return { gitDir: normalize(gitDir), gitConfigDir, isSubmodule } | ||
} catch (error) { | ||
return { error, gitDir: null } | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe it does not work properly because of this line. It takes path relative to node_modules or package.json (not sure), and from this going up a directory. I think it should be something like
const gitConfigDir = path.resolve(gitDir + '/' + dotGit.replace(/^gitdir: /, ''))
but probably better written
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for catching this. I tested it myself locally and the path in
.git
seems to be relative to thegitDir
, so that would be better indeed.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one more thing, in my case
dotGit.replace(/^gitdir: /, '')
contains new line at the end, so it tries to read frompath/path/path
MERGE_MSG
when i add .trim() after replace it reads from proper path.
With this change and above change to path it works great in my case 🥇
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those following newlines could use some trimming in other places as well, let me fix... thanks for testing!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done! Maybe a re-test just in case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works great now