Get correct absolute path of changed files when .changeset/ is not in the root of the repository

[alizeait]

When creating absolute paths from changed files, we assume cwd is the repository root but that is not always the case like when .changesets is set in a subfolder. Instead we can ask git for the repository root and create correct absolute path of the files returned from git diff

[changeset-bot]

🦋 Changeset detected

Latest commit: 08770b0

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@changesets/cli	Patch
@changesets/git	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 08770b0:

Sandbox	Source
Vanilla	Configuration

[Andarist]

Ok, I think I understand what you are saying... but we would need a test case for this change, ideally one that would involve some changesets command, or at least a full user story that involves Changesets.

[alizeait]

Ok, I think I understand what you are saying... but we would need a test case for this change, ideally one that would involve some changesets command, or at least a full user story that involves Changesets.

Sure, I added a unit test that asserts the changed behavior. Not sure how adding a Changeset command test case would be beneficial here considering that the git functions are simply mocked in those test files. Would appreciate any pointers if you have something else in mind

[Andarist]

I was just wondering how exactly this has manifested in a bug in Changesets. Did changesets status report a wrong value~? Or maybe changesets add didn't list the packages correctly?

[alizeait]

Exactly, changesets add would not report the list of changed packages since changed files full paths were not correct. For example in a monorepo with this setup where .changeset folder is not in the monorepo root:

monorepo/
├── packages/
│   ├── .changeset/
│   │   └── config.json
│   ├── pkg-a/
│   │   └── package.josn
│   ├── pkg-b/
│   │   └── package.json
│   └── package.json
└── package.json

When running changesets add from /packages folder, let's say pkg-a/package.json was modified, the reported full path of that file would be:
/home/user/monorepo/packages/packages/pkg-a/package.json, notice the double /packages, this is because we have cwd as /home/user/monorepo/packages and changed file reported by git is packages/pkg-a/package.json (always relative repository root).
Now when we do path.resolve(cwd, 'packages/pkg-a/package.json') we get:
/home/user/monorepo/packages/packages/pkg-a/package.json which is not correct. So instead of using cwd in path.resolve, we can ask git for the repository root and always get the correct full path of the changed file.

[Andarist]

Is this a defensive check or perhaps this can sometimes happen for some legitimate reason?

[alizeait]

I doubt It'll fail, but if it does, fallback to the previous behavior

[Andarist]

according to the docs:

Show the (by default, absolute) path of the top-level directory of the working tree. If there is no working tree, report an error.

So I think that we should throw an error here:

Suggested change

	if (code !== 0) {
	return cwd;
	}
	if (code !== 0) {
	throw new Error(gitCmd.stderr.toString());
	}

Where for us this actually shouldn't ever throw, but this function is quite generic - so if we ever expose it then somebody could call it with cwd outside of a git repo.

I think that it's better to throw here to raise potential issues early, rather than to swallow the error and lead to undetermined behavior.

[alizeait]

Sounds reasonable