From cf8d96e0b26fa381c4e80b9a94b8e6ad4cc95d50 Mon Sep 17 00:00:00 2001 From: Michael Cho Date: Sun, 28 Mar 2021 04:04:07 -0700 Subject: [PATCH 1/2] cask install: fix APFS DMG eject to use physical disk --- Library/Homebrew/test/cask/installer_spec.rb | 24 ++++++++++++--- .../fixtures/cask/Casks/container-apfs-dmg.rb | 9 ++++++ .../support/fixtures/cask/container-apfs.dmg | Bin 0 -> 16822 bytes Library/Homebrew/unpack_strategy/dmg.rb | 29 +++++++++++++++--- 4 files changed, 53 insertions(+), 9 deletions(-) create mode 100644 Library/Homebrew/test/support/fixtures/cask/Casks/container-apfs-dmg.rb create mode 100644 Library/Homebrew/test/support/fixtures/cask/container-apfs.dmg diff --git a/Library/Homebrew/test/cask/installer_spec.rb b/Library/Homebrew/test/cask/installer_spec.rb index f205bd7074..00411028f6 100644 --- a/Library/Homebrew/test/cask/installer_spec.rb +++ b/Library/Homebrew/test/cask/installer_spec.rb @@ -16,13 +16,27 @@ describe Cask::Installer, :cask do expect(caffeine.config.appdir.join("Caffeine.app")).to be_a_directory end - it "works with dmg-based Casks" do - asset = Cask::CaskLoader.load(cask_path("container-dmg")) + [ + ["HFS+", "container-dmg"], + ["APFS", "container-apfs-dmg"], + ].each do |filesystem, cask| + it "works with #{filesystem} dmg-based Casks" do + asset = Cask::CaskLoader.load(cask_path(cask)) + diskutil_list_command = "diskutil list | grep '/dev'" - described_class.new(asset).install + sleep(1) + original_diskutil_list = `#{diskutil_list_command}` - expect(Cask::Caskroom.path.join("container-dmg", asset.version)).to be_a_directory - expect(asset.config.appdir.join("container")).to be_a_file + described_class.new(asset).install + + expect(Cask::Caskroom.path.join(cask, asset.version)).to be_a_directory + expect(asset.config.appdir.join("container")).to be_a_file + + sleep(2) + expect { system diskutil_list_command } + .to output(original_diskutil_list) + .to_stdout_from_any_process + end end it "works with tar-gz-based Casks" do diff --git a/Library/Homebrew/test/support/fixtures/cask/Casks/container-apfs-dmg.rb b/Library/Homebrew/test/support/fixtures/cask/Casks/container-apfs-dmg.rb new file mode 100644 index 0000000000..6fc863207b --- /dev/null +++ b/Library/Homebrew/test/support/fixtures/cask/Casks/container-apfs-dmg.rb @@ -0,0 +1,9 @@ +cask "container-apfs-dmg" do + version "1.2.3" + sha256 "0630aa1145e8c3fa77aeb6ec414fee35204e90f224d6d06cb23e18a4d6112a5d" + + url "file://#{TEST_FIXTURE_DIR}/cask/container-apfs.dmg" + homepage "https://brew.sh/container-apfs-dmg" + + app "container" +end diff --git a/Library/Homebrew/test/support/fixtures/cask/container-apfs.dmg b/Library/Homebrew/test/support/fixtures/cask/container-apfs.dmg new file mode 100644 index 0000000000000000000000000000000000000000..5ddbd80772982f15bca8f141ae05b9a6930cfccd GIT binary patch literal 16822 zcmeHOc|6ox8y_N4H%o~dnu*HN)hLv#*(xbnlgK_Xwvn+7F=@GNqHal1C?O$jQnGam zH$oIswvi>_4q0Z5W#+sy)*0ruy!CoN@B1--%$(mj&vSmy`99z0InVk1KI6|uczyks zKOsvwURz#YVyJyY$JFjAWbwQsuYc!bpI@?K@6sD9-JUI8r1>Y$Im4q4hX+b2`UW|; zROL1MD+GVdB=!ZkcD1=ECGQR|vCn|F#UDb+5n|P&+`RtU*edB~ao`%sFODC!Spv)p(#T{z1{@3m6mF6Gbeol%*N)yR>i+nmh7VpOWnn)-PD0!Z# z)0ZG_n$31=@i8EYm*O;zw>@_P0*MF~fYa7r9~f#?6$EK~z+3C;(427(^YD)cixXD5 zySXKErY=)`tn*BU*CF~vp`pXc`Yc%V5lh2}Cfx{`D4pbcm6dVaY%+XiMl0_$tufLL zS$czdVd-_5bEhxy-1tM_rt*>8Cg*^%R}?}R7w^-~4BXR(?mw@gp=}L;U7H*Ng9lU( z4Ck)q8ioe-+M2r<2Rlf%97Gsq=DA{$X-z#QmwunGyF<55?%Td-TbbeO9yP!F~ z8~eofipcdKK|JbZC&DEigp)W+bF3HeIScNs%d#K5%o%1*$)kdzk!4EfCZe@biD#rS zTWDQQcer`Ih=hr5wtDpt7&N=G^)?9appKfk+$8Cc5y-n+EV*)Od7~yOFU%%WR&(X` zochoMB&ZtJL|~>E++hlAI;ak>fboD$wPhzjC7-@S%{y|APxyF#rBTaygd-afNQk`D zoLHuu4+ph5Ha4%H6uGBfB}K;TMJH`-8u7}(H-~_(S*`(br~+uz!2ODah``A}%w_dz zW z6yc+M_P+&@>&e9mH1BRpQSQD#L#Q>ZXP$iyWl+a0oZe8gF|ljGd1ViQ^VeF&XEMm% zGCU9!5TlF(N#3QW;D4;8XZZY80((t0?heGGcyXX8Pox7#Rz%;FVov|ISJ`(5^_jrc z@BaN-n?U?HxH;c8xe}hY@O7$4y3at;+K*M8m_>(GlM3NO5pQ*D|7P#Gs+aftUhU>1 z6Xc}xLl@R`dqZOIuiMi@6U+-6!WC+sYzwHBhz;mT$T+dW1;q=e!Vf1dSBh z_A>nmQqdR^Vq!(pt|$cfeL0QVP)tu_;wOMN84S$;;HcyifF1F$3sm{tzlIX@N=;UZzR(1E}81;ISReR-B_L(CQHM@8YzLokW9X8WaSbrNxSL*4S2ZUnO3c z7YJ8Tr~?|*`quzknXSvC#b5<3MHR|=7CgyZk=0#`CT-2ZyZ~tg5M6%GwSFG5fS&?} zMRIrw_j5>I{+-QvntC~y43Q#1NDK31W$iLNOoPUjd_dg8g>W%ljU~(AiTW(W5=p{!TU3; z(wHX6~<(Taidxm@b9JuGP@7tn4Yy*%Vm!c_6N;u9x|B^;>yCh%i17Sa<4(Wan8yw5&w{yxH`R_Px zhy~ufRs)Kt&fU}`B03O{L6h7oR;V}KzlypAuxQJ)!2JQ}u8JXms~jwZ2-nss63|6$ zo~aaCCTnRz2lPFRM`rmf@vHMxv=u?kfYUG#RbfC4Ot+_2X;$kp7}Y4 zfN!&%*@UrlGi@2u%>OwuwlzF;4FhjGQf%@Le7?OSE^~)dg6YDGce43v9V~-D3O#dlAWlm=NIvawxN<9?@0z&h4N1O zqZg-f`|?9}*(w0N{P6h9xvbdnnC6#NKpkD*m{DaBaQk5nRpU`n!43*Iim`@@+?)n& z{!KyWWRUI(kwfCwa7r-4*DUbvet!}%)0%L{{Z^TA;Sl@{vWg7u#S8;narV6@wQHvV zDtDYC#-G&J8EZ}A-L%yTzr4H62_ii(?^dEy?;RX%ua}N^KfSKKdMM<00ypI>(C>|r zZW^gy7`zYLPUx!fF$)uJ)93)r`L_V@cRMFAjo&6)8@FYyYXC?0M_ajfq+S=ToH&b!FHM{yUKziINX7p?YxD|pUyMu0jlA!Lk+4t}u zv8cM?Zn6y8b;M?|5ZHZWlLaQSjU?y2H_g7Z zCjq@Xd~g(iSai!5tsWfY)E}&|izLsm+i?VL@^L^)9MVBoF;Ltp4f1&5mc! zPrq9~@E{yJNK)~oHat?RjSTW?6YGK57YxT%TQhO(@>X`EOfetaQ#_f9vwnvizH+Tn z!GD#3DAW=(-QO!iXd5;`4b;6|9(5^)fa?%*K|&QfEJ3ctX#-@Eadz~X2U0d42ogKa z$N$~+Ycl`lwjcuT(=OK?*Y|aR^0JiGGU-`Cyb&JN&xC+)eMIb&M^9_`oubUc%HJS5 zH!2ti)<#>N80LQw5U3nq!`J>QJ9qW|+*|K1p-T{&m9+|r~!3t56LAfL5=TYxF9HzomW`p^lD`>EuF>%c{EO0!(<)D!+^DsQr2VAdgW0 za8=XLNuZyN8UaS)2G86G1hvFQS*Z;SQ*$JyZ}AYt2KfxcivduVAXJx)#&P;ltFIFL zy6fcc+L!fLq1%`_)_75nTxK2&+G2*QaG4`peNI%wK7I>|x`E;_mOh^!MsVK7oQFf&B@Kxc-Qo9oiUWqpAP8f5sybc+d-H_#6Cdy~iTXK8 z7@NMD3SAEJM5CcxH&rTy+NwL%+PC0rI~1tkw)X+$2d*DpcSBOkcmGoDr*XA*b$>#+ z4>2*iFYZ{l-!-vTVcN82T*rgZ>!^iEC+1`Ip*hO)RjiDp>r4}lp8YeB@z9vRivQM; zsFQZaS>2USu;;d=hWDoy2?eKz+QwGr_1-HaJv;DB;`Ev36NRVq{K$n_ZwfG}$}o)c-fKPo%Z$V&Ub1 zgV6JZL5m*MZ9}(EFv!6u8_V2-oiX@&f4{{;VP0vuCuQ;SYd{Ol#SJCA!7U^5x!3S- z$ZCgu&zyYm-2PBAh`o@Id$sMrZyF<_jd{5NkEtcDfi9a`gAk`v3dz{gwCBx4)R|w5 zr_-XCTU6bt>^U@`Bb0iwQ-WH)`(lGU6~84w3H)Y@CE~Xja3#J9lKFI=ofyz}-bQt^ z8wwnB^l)YJy{9p!7k6fc%od55lFsN9VnrAjC;)*U&L`ysKRY7KtKI}eKW$1ZsdnFM zvP|5-%|pyXgRhg&{>tTfsEllU4yO(s|}tOc`nvpA}#m!2GRmkzNWYMLi4_!wu~GF_g}JV{QRuKH6q32 z-%TFzX@|y(f_sM=KLnJMVrpco#B@~o2|!|9*yF|vCL{yFtIQkSPMZ+ax~RU-kJ4i* zpa+9qfYlA<<&er$)lIQ%s&5h~*g_@UL=p)Q2pR_GYrRDn!BbM?@RGnsr>LAF0i1bf zygSJ2uBYGwUE9Mf_yKWGQTQVYz{KJoJ$@LA;<~nu0Q6J((lR~r0?R9&F1#6zf78DJ z=PTLSyyP_%dvAcIf6 z&wLCILCSot4&MkLY)nbVkFJGw_=v0kC4YfZynpDvLgeX9$5Bsna<=tzouood$kj`) zF@lkFam&Mja1uBhUGjlU>R7j}mI?ry#dGOo#zjxtA?Jn|A)G^F4vo10nTvwCD42_a zxhVL*iUPUCoK$F;fHH~pLbUcCkEW4mQ4gq3Pz}+Ro{x}w@3E;kZk_Z(y@7^fdmZI3 zr_E2%%&K1n{~*@2R%(Mkq^s1zZR_&^ns3=EmzL}m40r08n8Ijtz~QT;VHH48yalpzTDvwU!b6{<|2Am~4)sHEEKbj>Y82c3(K{qcnLv))<3)QeoQY z#HHG(fGT{-X`|%Q(_!@JIE!T0v+({;q!Z1Gu~7MslYN_^DQr8!&XLA)DMK%$yPX#X z#?Yq>TmKV?KI+O*oWDTyDWM6E%=jI5;TX}iT(;{O(3$du&)0lDLv?l)zK`hJ*Sb#H zYkFXIh|Vf;{sYe$So2Lxnq7_?+Ie{+y=i+RjGm;Kxo&D_?}nh|^bJ3O>QjWAYSFwl z&a3g9&NMXXQ>F$J)g2w3UEqF9<1;R1m$@@;&#Hsbq$#RzXPMJSAz@UQoDj__r;qP& zEEOg-H8sB{r+-IEr&ImkAk^$J9ws$f&Dm`ALePmV|Kr5go8s&?27sp#`zsu!D~`Kt zEQd4B^iS=t`FMuj>?({gk&X^)VvnLVWztTDs?LyA4j(IKrW9r@Z?xlkh`r(4Yn3na zX7>2&h(x=>XvK&VV!|R#!>uMXCM*0nll3PM)rBieH0i(NkdLKkraFaDilIHOFveu_ z02h1vsTg4pn`(Z|sZ)~Ut=U!3rJFxmcC%!%!KhJFCOJ-)-DYEG(Sgtk7&l#0Q*Nv^ zp`hzNfsVG8*`&}|gXz37u5iF^LdZmm$xjpAn)K+Q?}`(57$49wTstYkx-qVx_B}x) z|LsBat>m4(aGD6B@$EqIM}tVqh#`wHg~~Xz<3aSjB|U3*G51dMHuE5>tJzhU+74LR zLJJUjH*1qvZU;aA?5gnXO#U)&W{Mboh2Uj1@(X;AA{l4)h@Op}}6X|NykIcMg z8e;9|afPwX;s2_B*PNccM*$dxWQx}YlbeI)Y}N0I-_GlQk59A9_lbux3_Kt2Qig5% zaiSYd5&a7Z{Cn$pl+r=_ZBsL|42>z0upFM*IlBr|2UXTYpO(NargDKPtApzJ^WLwm z;d@Gvjo(M~?;z3?!&TvvkK0F|f_l24y(gc8{`5!j|NPv1CL8{_=W7_x(41440SSwa zIh0s{GNbE1ffB9uW5}idi90YaW9|;jc(A^jLy0vm%)LL3(&!Ad9cl6%>Oa4Z`;NC1 zOnWD*{cfUf>UZ>AwmOe1$_+#N2RsJ^!cO~vu+hI}eqg3E`~DYtChUgV{}-*L0}JD5 TUrYWk+CabcxNi-$8148M`ANZ> literal 0 HcmV?d00001 diff --git a/Library/Homebrew/unpack_strategy/dmg.rb b/Library/Homebrew/unpack_strategy/dmg.rb index 5f2fea6825..866d18208e 100644 --- a/Library/Homebrew/unpack_strategy/dmg.rb +++ b/Library/Homebrew/unpack_strategy/dmg.rb @@ -87,10 +87,31 @@ module UnpackStrategy return unless path.exist? if tries > 1 - system_command! "diskutil", - args: ["eject", path], - print_stderr: false, - verbose: verbose + disk_info = system_command!( + "diskutil", + args: ["info", "-plist", path], + print_stderr: false, + verbose: verbose, + ) + + # For HFS, just use + # For APFS, find the corresponding to + eject_paths = if disk_info.success? + disk_info.plist + .fetch("APFSPhysicalStores", []) + .map { |store| store["APFSPhysicalStore"] } + .compact + .presence || [path] + else + [path] + end + + eject_paths.each do |eject_path| + system_command! "diskutil", + args: ["eject", eject_path], + print_stderr: false, + verbose: verbose + end else system_command! "diskutil", args: ["unmount", "force", path], From 1a398f9aafa04502a82ae48703b4e6ae1905cb81 Mon Sep 17 00:00:00 2001 From: Michael Cho Date: Mon, 29 Mar 2021 23:17:21 -0700 Subject: [PATCH 2/2] cask install: increase APFS test sleep to avoid slow eject fail --- Library/Homebrew/test/cask/installer_spec.rb | 38 +++++++++++--------- Library/Homebrew/unpack_strategy/dmg.rb | 14 +++----- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/Library/Homebrew/test/cask/installer_spec.rb b/Library/Homebrew/test/cask/installer_spec.rb index 00411028f6..e9da105157 100644 --- a/Library/Homebrew/test/cask/installer_spec.rb +++ b/Library/Homebrew/test/cask/installer_spec.rb @@ -16,27 +16,31 @@ describe Cask::Installer, :cask do expect(caffeine.config.appdir.join("Caffeine.app")).to be_a_directory end - [ - ["HFS+", "container-dmg"], - ["APFS", "container-apfs-dmg"], - ].each do |filesystem, cask| - it "works with #{filesystem} dmg-based Casks" do - asset = Cask::CaskLoader.load(cask_path(cask)) - diskutil_list_command = "diskutil list | grep '/dev'" + it "works with HFS+ dmg-based Casks" do + asset = Cask::CaskLoader.load(cask_path("container-dmg")) - sleep(1) - original_diskutil_list = `#{diskutil_list_command}` + described_class.new(asset).install - described_class.new(asset).install + expect(Cask::Caskroom.path.join("container-dmg", asset.version)).to be_a_directory + expect(asset.config.appdir.join("container")).to be_a_file + end - expect(Cask::Caskroom.path.join(cask, asset.version)).to be_a_directory - expect(asset.config.appdir.join("container")).to be_a_file + it "works with APFS dmg-based Casks" do + asset = Cask::CaskLoader.load(cask_path("container-apfs-dmg")) + diskutil_list_command = "diskutil list | grep '/dev'" - sleep(2) - expect { system diskutil_list_command } - .to output(original_diskutil_list) - .to_stdout_from_any_process - end + sleep 5 + original_diskutil_list = `#{diskutil_list_command}` + + described_class.new(asset).install + + expect(Cask::Caskroom.path.join("container-apfs-dmg", asset.version)).to be_a_directory + expect(asset.config.appdir.join("container")).to be_a_file + + sleep 5 + expect { system diskutil_list_command } + .to output(original_diskutil_list) + .to_stdout_from_any_process end it "works with tar-gz-based Casks" do diff --git a/Library/Homebrew/unpack_strategy/dmg.rb b/Library/Homebrew/unpack_strategy/dmg.rb index 866d18208e..f758c1a78c 100644 --- a/Library/Homebrew/unpack_strategy/dmg.rb +++ b/Library/Homebrew/unpack_strategy/dmg.rb @@ -96,15 +96,11 @@ module UnpackStrategy # For HFS, just use # For APFS, find the corresponding to - eject_paths = if disk_info.success? - disk_info.plist - .fetch("APFSPhysicalStores", []) - .map { |store| store["APFSPhysicalStore"] } - .compact - .presence || [path] - else - [path] - end + eject_paths = disk_info.plist + .fetch("APFSPhysicalStores", []) + .map { |store| store["APFSPhysicalStore"] } + .compact + .presence || [path] eject_paths.each do |eject_path| system_command! "diskutil",