feat: support yarnpkg as an alias for yarn by rytmis · Pull Request #1670 · volta-cli/volta

rytmis · 2024-01-28T09:30:27Z

My attempt at implementing #1391. This is my first Rust code aside from a helloworld example, so you know, it mightn't be perfect yet.

I tested (and wrote) this on macOS Sonoma 14.3 by switching between the 1.1.1 release and this dev build, and it seems to work, but I'm not 100% sure on how to reliably verify it. I bumped the version to 1.1.2 so that the dev install script would let me switch, not sure if I was supposed to do that. :)

rwjblue

This seems totally on the right path! Thanks for working on it!!

I think there are a few things we'd still need to do:

Ensure we create the yarnpkg shim (will require changes in the installer I think?)

volta/dev/unix/install.sh.in

Line 94 in 889ff97

ln -s "${INSTALL_DIR}"/shim "${INSTALL_DIR}"/bin/yarn

volta/dev/unix/volta-install-legacy.sh

Line 77 in 889ff97

local main_shims=( node npm npx yarn )

volta/wix/main.wxs

Lines 157 to 172 in 889ff97

    
           <Component Id='yarnBinary' Guid='*' Win64='$(var.Win64)'> 
        
               <File 
        
                   Id='yarnEXE' 
        
                   Name='yarn.exe' 
        
                   DiskId='1' 
        
                   Source='target\release\volta-shim.exe' 
        
                   KeyPath='yes'/> 
        
           </Component> 
        
           <Component Id='yarnScript' Guid='*' Win64='$(var.Win64)'> 
        
               <File 
        
                   Id='yarnCMD' 
        
                   Name='yarn.cmd' 
        
                   DiskId='1' 
        
                   Source='wix\shim.cmd' 
        
                   KeyPath='yes'/> 
        
           </Component>

Add some tests so we don't accidentally break this new feature. I'm not 100% sure where to add the tests. Maybe @chriskrycho or @charlespierce have an idea...

rwjblue · 2024-01-28T14:00:12Z

Cargo.toml

 [package]
 name = "volta"
-version = "1.1.1"
+version = "1.1.2"


In general, we only bump the version when we do releases.

rwjblue

Also need to make sure volta run knows how to handle this correctly. I think that means adding another match arm here:

volta/crates/volta-core/src/run/mod.rs

Line 99 in 889ff97

Some("yarn") => yarn::command(args, session),

Some tests are here (you could probably add a couple that call volta run --yarn=1.7.71 yarnpkg --version or something):

volta/tests/acceptance/volta_run.rs

Lines 357 to 433 in 889ff97

    
           #[test] 
        
           fn command_line_yarn_1() { 
        
               let s = sandbox() 
        
                   .node_available_versions(NODE_VERSION_INFO) 
        
                   .distro_mocks::<NodeFixture>(&NODE_VERSION_FIXTURES) 
        
                   .yarn_1_available_versions(YARN_1_VERSION_INFO) 
        
                   .distro_mocks::<Yarn1Fixture>(&YARN_1_VERSION_FIXTURES) 
        
                   .env(VOLTA_LOGLEVEL, "debug") 
        
                   .build(); 
        
               assert_that!( 
        
                   s.volta("run --node 10.99.1040 --yarn 1.7.71 yarn --version"), 
        
                   execs() 
        
                       .with_status(ExitCode::Success as i32) 
        
                       .with_stderr_contains("[..]Yarn: 1.7.71 from command-line configuration") 
        
               ); 
        
           } 
        
           #[test] 
        
           fn command_line_yarn_3() { 
        
               let s = sandbox() 
        
                   .node_available_versions(NODE_VERSION_INFO) 
        
                   .distro_mocks::<NodeFixture>(&NODE_VERSION_FIXTURES) 
        
                   .yarn_1_available_versions(YARN_1_VERSION_INFO) 
        
                   .yarn_berry_available_versions(YARN_BERRY_VERSION_INFO) 
        
                   .distro_mocks::<Yarn1Fixture>(&YARN_1_VERSION_FIXTURES) 
        
                   .distro_mocks::<YarnBerryFixture>(&YARN_BERRY_VERSION_FIXTURES) 
        
                   .env(VOLTA_LOGLEVEL, "debug") 
        
                   .build(); 
        
               assert_that!( 
        
                   s.volta("run --node 10.99.1040 --yarn 3.7.71 yarn --version"), 
        
                   execs() 
        
                       .with_status(ExitCode::Success as i32) 
        
                       .with_stderr_contains("[..]Yarn: 3.7.71 from command-line configuration") 
        
               ); 
        
           } 
        
           #[test] 
        
           fn inherited_yarn_1() { 
        
               let s = sandbox() 
        
                   .node_available_versions(NODE_VERSION_INFO) 
        
                   .distro_mocks::<NodeFixture>(&NODE_VERSION_FIXTURES) 
        
                   .yarn_1_available_versions(YARN_1_VERSION_INFO) 
        
                   .distro_mocks::<Yarn1Fixture>(&YARN_1_VERSION_FIXTURES) 
        
                   .package_json(&package_json_with_pinned_node_yarn("10.99.1040", "1.2.42")) 
        
                   .env(VOLTA_LOGLEVEL, "debug") 
        
                   .build(); 
        
               assert_that!( 
        
                   s.volta("run --node 10.99.1040 yarn --version"), 
        
                   execs() 
        
                       .with_status(ExitCode::Success as i32) 
        
                       .with_stderr_contains("[..]Yarn: 1.2.42 from project configuration") 
        
               ); 
        
           } 
        
           #[test] 
        
           fn inherited_yarn_3() { 
        
               let s = sandbox() 
        
                   .node_available_versions(NODE_VERSION_INFO) 
        
                   .distro_mocks::<NodeFixture>(&NODE_VERSION_FIXTURES) 
        
                   .yarn_1_available_versions(YARN_1_VERSION_INFO) 
        
                   .yarn_berry_available_versions(YARN_BERRY_VERSION_INFO) 
        
                   .distro_mocks::<Yarn1Fixture>(&YARN_1_VERSION_FIXTURES) 
        
                   .distro_mocks::<YarnBerryFixture>(&YARN_BERRY_VERSION_FIXTURES) 
        
                   .package_json(&package_json_with_pinned_node_yarn("10.99.1040", "3.2.42")) 
        
                   .env(VOLTA_LOGLEVEL, "debug") 
        
                   .build(); 
        
               assert_that!( 
        
                   s.volta("run --node 10.99.1040 yarn --version"), 
        
                   execs() 
        
                       .with_status(ExitCode::Success as i32) 
        
                       .with_stderr_contains("[..]Yarn: 3.2.42 from project configuration") 
        
               ); 
        
           }

rytmis · 2024-01-28T15:49:14Z

Interesting! Something gave me a yarnpkg symlink under .volta/bin--but I'll take a look at your links a bit later and see if I can figure what's what.

rwjblue · 2024-01-28T16:56:06Z

Something gave me a yarnpkg symlink under .volta/bin--but I'll take a look at your links a bit later and see if I can figure what's what.

Most likely that was done by volta setup (which your changes to get_shim_list_deduped in volta/crates/volta-core/src/shim.rs do).

rytmis · 2024-01-28T21:30:32Z

Also need to make sure volta run knows how to handle this correctly. I think that means adding another match arm here:

Did I mess something up in the PR? I thought I handled it here:

volta/crates/volta-core/src/run/mod.rs

Lines 99 to 100 in 994925b

    
           Some("yarn") => yarn::command(args, session, "yarn"), 
        
           Some("yarnpkg") => yarn::command(args, session, "yarnpkg"),

And here:

volta/crates/volta-core/src/run/yarn.rs

Line 22 in 994925b

yarn_executable: &str,

volta/crates/volta-core/src/run/yarn.rs

Line 40 in 994925b

Ok(ToolCommand::new(yarn_executable, args, platform, ToolKind::Yarn).into())

As for the tests--do you prefer a separate test that duplicates the setup, or would a second assertion in the test do?

And finally: yarn 3 doesn't seem to ship the yarnpkg command. That results in a NoPlatform error, which is reported as a missing node installation. The NoPlatform error should probably include the name of the missing component? Should I try to handle the yarn 3 case gracefully, or would the error suffice, if the message was correct?

rwjblue · 2024-01-28T23:44:08Z

Did I mess something up in the PR? I thought I handled it here:

Nope, you didn't miss anything. I did! 😸 I was jumping between main and your PR, and must have gotten confused about which branch I was on. 🤦

As for the tests--do you prefer a separate test that duplicates the setup, or would a second assertion in the test do?

I think additional assertions are fine (though I'll defer to @chriskrycho / @charlespierce if they have a strong preference).

And finally: yarn 3 doesn't seem to ship the yarnpkg command.

Oof. That makes things a bit rough. I do worry about the situation where someone might check (e.g. in a script do which yarnpkg as a way to switch or something), but IMHO that's not a great way to test anyways so probably not a giant issue for us here.

That results in a NoPlatform error, which is reported as a missing node installation. The NoPlatform error should probably include the name of the missing component? Should I try to handle the yarn 3 case gracefully, or would the error suffice, if the message was correct?

Yeah, I think we'll need to tweak this. I'd prefer to have a custom error specifically for this scenario (usage of yarnpkg with yarn@3+) (and we can throw another specific test for that (here).

charlespierce · 2024-02-01T20:18:25Z

Regarding the Yarn 3 / yarnpkg missing issues: It looks like the Yarn 1 version of yarnpkg was only ever a pure passthrough to yarn—would it make sense for Volta to treat it the same way? That is, we don't actually change the yarn::command function to take in the executable, but continue to always execute yarn, regardless of whether it was actually called using yarn or yarnpkg?

That would mean that yarnpkg continues to work on Yarn 3, even though it isn't actually shipped with it, but it would simplify the code a little and remove the issues with the specific version.

rytmis · 2024-02-04T10:32:32Z

That makes more sense than any other alternative I can think of, yes. I reverted the yarn::command change and combined the yarn and yarnpkg arms of the match into one.

rytmis · 2024-02-05T22:18:25Z

Anything else I should do to get this finalized?

I just verified that this works on Windows--I went down this rabbit hole because I noticed that the lack of a yarnpkg shim to Volta was causing issues with caniuse-lite's update command, and lo and behold, that use case works now. :D

rwjblue

Seems good to me!

@charlespierce / @chriskrycho - Any other changes needed from y'alls perspective?

chriskrycho

Nope, seems excellent, let's do it! (And sorry for the delay!)

feat: support yarnpkg as an alias for yarn

eabfd7c

rwjblue reviewed Jan 28, 2024

View reviewed changes

rytmis added 2 commits January 28, 2024 18:14

style: reformatted code

d6bc325

chore: revert version bump

994925b

feat: create yarnpkg shim in install scripts

525daa3

chore: recert version bump from Cargo.lock too

e69f29b

chore: add yarnpkg alias to wix installer

b4a2c17

rytmis added 2 commits February 4, 2024 12:33

test: add assertion for yarnpkg

cb7cbc5

feat: map yarnpkg to yarn, so it works across yarn versions

7544015

rwjblue approved these changes Feb 18, 2024

View reviewed changes

chriskrycho approved these changes Mar 14, 2024

View reviewed changes

chriskrycho added the enhancement label Mar 14, 2024

chriskrycho linked an issue Mar 14, 2024 that may be closed by this pull request

Volta should create a shim for yarnpkg #1391

Closed

chriskrycho merged commit ddfdda9 into volta-cli:main Mar 14, 2024

rytmis deleted the feature/yarnpkg-shim branch March 14, 2024 11:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: support yarnpkg as an alias for yarn#1670

feat: support yarnpkg as an alias for yarn#1670
chriskrycho merged 8 commits intovolta-cli:mainfrom
rytmis:feature/yarnpkg-shim

rytmis commented Jan 28, 2024

Uh oh!

rwjblue left a comment

Uh oh!

rwjblue Jan 28, 2024

Uh oh!

rwjblue left a comment

Uh oh!

rytmis commented Jan 28, 2024

Uh oh!

rwjblue commented Jan 28, 2024 •

edited

Loading

Uh oh!

rytmis commented Jan 28, 2024

Uh oh!

rwjblue commented Jan 28, 2024

Uh oh!

charlespierce commented Feb 1, 2024

Uh oh!

rytmis commented Feb 4, 2024

Uh oh!

rytmis commented Feb 5, 2024

Uh oh!

rwjblue left a comment

Uh oh!

chriskrycho left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	<Component Id='yarnBinary' Guid='*' Win64='$(var.Win64)'>
	<File
	Id='yarnEXE'
	Name='yarn.exe'
	DiskId='1'
	Source='target\release\volta-shim.exe'
	KeyPath='yes'/>
	</Component>
	<Component Id='yarnScript' Guid='*' Win64='$(var.Win64)'>
	<File
	Id='yarnCMD'
	Name='yarn.cmd'
	DiskId='1'
	Source='wix\shim.cmd'
	KeyPath='yes'/>
	</Component>

	#[test]
	fn command_line_yarn_1() {
	let s = sandbox()
	.node_available_versions(NODE_VERSION_INFO)
	.distro_mocks::<NodeFixture>(&NODE_VERSION_FIXTURES)
	.yarn_1_available_versions(YARN_1_VERSION_INFO)
	.distro_mocks::<Yarn1Fixture>(&YARN_1_VERSION_FIXTURES)
	.env(VOLTA_LOGLEVEL, "debug")
	.build();

	assert_that!(
	s.volta("run --node 10.99.1040 --yarn 1.7.71 yarn --version"),
	execs()
	.with_status(ExitCode::Success as i32)
	.with_stderr_contains("[..]Yarn: 1.7.71 from command-line configuration")
	);
	}

	#[test]
	fn command_line_yarn_3() {
	let s = sandbox()
	.node_available_versions(NODE_VERSION_INFO)
	.distro_mocks::<NodeFixture>(&NODE_VERSION_FIXTURES)
	.yarn_1_available_versions(YARN_1_VERSION_INFO)
	.yarn_berry_available_versions(YARN_BERRY_VERSION_INFO)
	.distro_mocks::<Yarn1Fixture>(&YARN_1_VERSION_FIXTURES)
	.distro_mocks::<YarnBerryFixture>(&YARN_BERRY_VERSION_FIXTURES)
	.env(VOLTA_LOGLEVEL, "debug")
	.build();

	assert_that!(
	s.volta("run --node 10.99.1040 --yarn 3.7.71 yarn --version"),
	execs()
	.with_status(ExitCode::Success as i32)
	.with_stderr_contains("[..]Yarn: 3.7.71 from command-line configuration")
	);
	}

	#[test]
	fn inherited_yarn_1() {
	let s = sandbox()
	.node_available_versions(NODE_VERSION_INFO)
	.distro_mocks::<NodeFixture>(&NODE_VERSION_FIXTURES)
	.yarn_1_available_versions(YARN_1_VERSION_INFO)
	.distro_mocks::<Yarn1Fixture>(&YARN_1_VERSION_FIXTURES)
	.package_json(&package_json_with_pinned_node_yarn("10.99.1040", "1.2.42"))
	.env(VOLTA_LOGLEVEL, "debug")
	.build();

	assert_that!(
	s.volta("run --node 10.99.1040 yarn --version"),
	execs()
	.with_status(ExitCode::Success as i32)
	.with_stderr_contains("[..]Yarn: 1.2.42 from project configuration")
	);
	}

	#[test]
	fn inherited_yarn_3() {
	let s = sandbox()
	.node_available_versions(NODE_VERSION_INFO)
	.distro_mocks::<NodeFixture>(&NODE_VERSION_FIXTURES)
	.yarn_1_available_versions(YARN_1_VERSION_INFO)
	.yarn_berry_available_versions(YARN_BERRY_VERSION_INFO)
	.distro_mocks::<Yarn1Fixture>(&YARN_1_VERSION_FIXTURES)
	.distro_mocks::<YarnBerryFixture>(&YARN_BERRY_VERSION_FIXTURES)
	.package_json(&package_json_with_pinned_node_yarn("10.99.1040", "3.2.42"))
	.env(VOLTA_LOGLEVEL, "debug")
	.build();

	assert_that!(
	s.volta("run --node 10.99.1040 yarn --version"),
	execs()
	.with_status(ExitCode::Success as i32)
	.with_stderr_contains("[..]Yarn: 3.2.42 from project configuration")
	);
	}

Conversation

rytmis commented Jan 28, 2024

Uh oh!

rwjblue left a comment

Choose a reason for hiding this comment

Uh oh!

rwjblue Jan 28, 2024

Choose a reason for hiding this comment

Uh oh!

rwjblue left a comment

Choose a reason for hiding this comment

Uh oh!

rytmis commented Jan 28, 2024

Uh oh!

rwjblue commented Jan 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rytmis commented Jan 28, 2024

Uh oh!

rwjblue commented Jan 28, 2024

Uh oh!

charlespierce commented Feb 1, 2024

Uh oh!

rytmis commented Feb 4, 2024

Uh oh!

rytmis commented Feb 5, 2024

Uh oh!

rwjblue left a comment

Choose a reason for hiding this comment

Uh oh!

chriskrycho left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

rwjblue commented Jan 28, 2024 •

edited

Loading