Skip to content
This repository was archived by the owner on Sep 17, 2023. It is now read-only.

fix: use pack source instead of hardlink #105

Merged

Conversation

bitgopatmcl
Copy link
Contributor

@bitgopatmcl bitgopatmcl commented Mar 17, 2022

In order to avoid race conditions from multiple build jobs clobbering the pack file, instead of creating an archive and hardlinking it around, just pack up packages directly to their intended destinations.

@bitgopatmcl bitgopatmcl marked this pull request as draft March 17, 2022 15:49
@bitgopatmcl bitgopatmcl force-pushed the pack-from-relative-dir branch from b21ef00 to ea63d1c Compare March 17, 2022 15:51
@bitgopatmcl bitgopatmcl marked this pull request as ready for review March 17, 2022 15:51
@bitgopatmcl bitgopatmcl force-pushed the pack-from-relative-dir branch from ea63d1c to 4383106 Compare March 17, 2022 16:04
.to_str()
.expect("npm pack filename is not UTF-8 encodable")
.expect("source package path is not UTF-8 encodable")
Copy link
Contributor

Choose a reason for hiding this comment

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

expect messages start with an uppercase letter in this project, except for npm because npm is a proper noun 😅

Not blocking, can follow-up

Copy link
Contributor

@EricCrosson EricCrosson 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 to me, would prefer a deeper writeup in the commit message about exactly where/how the race condition is produced so we can leave a sufficient trail of breadcrumbs to avoid past mistakes

@EricCrosson EricCrosson merged commit a9de18f into typescript-tools:master Mar 17, 2022
@bitgopatmcl bitgopatmcl deleted the pack-from-relative-dir branch March 17, 2022 17:06
@github-actions
Copy link

🎉 This PR is included in version 3.0.8 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

3 participants