-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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(release): Support path offset from git root to Nx workspace #28168
fix(release): Support path offset from git root to Nx workspace #28168
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
☁️ Nx Cloud ReportCI is running/has finished running commands for commit eb5bca1. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 4 targets
Sent with 💌 from NxCloud. |
dd83744
to
eefb586
Compare
Hi @xiongemi @leosvelperez. Do you need anything else for the review of this PR? Please let me know if I need to add anything. We are currently relying on a custom monkey patch using patch-package to make it work in our environment. |
This is a bit of a blocker for us too, would love to see it released.. |
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 a lot for this @gionkunz and I'm sorry for the delay in getting back to you. I think we can avoid a lot of the changes here by acknowledging that the git absolute root and its offset are basically as constant as the workspaceRoot itself.
Therefore, in git.ts
I would simply do that following:
// ...
// ... existing parseGitCommit() function here (with the one tweak to leverage replaceGitRootPathDiff() in the reduce as you have already)
// ...
/**
* Lazily compute the offset from the workspace root to the git root, if any, one time,
* it is not something that changes during the execution of a command.
*/
let workspaceGitRootOffset: string;
function replaceGitRootPathDiff(file: string) {
if (!workspaceGitRootOffset) {
let absoluteGitRoot: string;
// This check is probably debatable, we could probably just always run the getAbsoluteGitRoot tbf...
if (existsSync(join(workspaceRoot, '.git'))) {
absoluteGitRoot = workspaceRoot;
} else {
absoluteGitRoot = getAbsoluteGitRoot();
}
workspaceGitRootOffset = getWorkspaceGitRootOffset(
absoluteGitRoot,
workspaceRoot
);
}
if (
!workspaceGitRootOffset ||
!file.startsWith(workspaceGitRootOffset)
) {
return file;
}
return file.replace(workspaceGitRootOffset, '');
}
// (no need to export this util any more)
function getAbsoluteGitRoot() {
try {
return execSync('git', ['rev-parse', '--show-toplevel']).trim();
} catch (e) {
throw new Error(`Could not determine git root`);
}
}
With your getWorkspaceGitRootOffset()
function and unit tests remaining the same.
I think that gets us everything we need without changing any other files or function signatures right?
@JamesHenry Even better! Thanks! |
This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request. |
@JamesHenry @AgentEnder
Current Behavior
Currently Nx Release does not support the use-case where the Nx Workspace is within a nested folder under the Git root directory. As it's quite common to have a backend,infra,frontend separation on the repo top directory and then only initialize the Nx Workspace in the frontend directory, this is probably needed in many projects.
Also, see issue #27995
Expected Behavior
The version and changelog commands both should support workspaces that are nested within sub-folders of a Git repository.
Related Issue(s)
Fixes #27995