Skip to content
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

chore: fire warnings on too long sync paths #4582

Merged
merged 6 commits into from
Jun 9, 2023

Conversation

vvagaytsev
Copy link
Collaborator

What this PR does / why we need it:
Add some warnings on too-long sync paths to make the error details more informative. See individual commits for details.

Which issue(s) this PR fixes:

Mitigates #4527

Special notes for your reviewer:
I skipped the FAQ docs update. The warnings are visible well enough to users. And this problem occurs not that often for be a Frequently asked question :)

The commits will be squashed before the merge.

Use action kind instead of "Module"
The `MUTAGEN_DATA_DIRECTORY` env variable has some implicit length limitations. Making it too long can cause mutagen failures with code `1` because of the Unix socket path length limit.
@vvagaytsev vvagaytsev requested a review from a team June 9, 2023 10:34
core/src/mutagen.ts Outdated Show resolved Hide resolved
core/src/mutagen.ts Outdated Show resolved Hide resolved
@vvagaytsev vvagaytsev requested review from Orzelius and a team June 9, 2023 11:08
@vvagaytsev vvagaytsev dismissed Orzelius’s stale review June 9, 2023 12:33

The changes have been addressed

@vvagaytsev vvagaytsev requested review from Orzelius and a team and removed request for a team and Orzelius June 9, 2023 12:34
core/src/mutagen.ts Outdated Show resolved Hide resolved
Co-authored-by: Shumail Mohyuddin <[email protected]>
@vvagaytsev vvagaytsev requested a review from shumailxyz June 9, 2023 12:58
Copy link
Contributor

@shumailxyz shumailxyz left a comment

Choose a reason for hiding this comment

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

Looks good. Thank you 👍

@@ -127,7 +127,7 @@ export async function syncToBuildSync(params: SyncToSharedBuildSyncParams) {
log,
key,
logSection: action.key(),
sourceDescription: `Module ${action.name} build path`,
sourceDescription: `${action.kind} ${action.name} build path`,
Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch 👍

@vvagaytsev vvagaytsev merged commit 67e7d2d into main Jun 9, 2023
@vvagaytsev vvagaytsev deleted the fix-sync-with-long-paths branch June 9, 2023 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants