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

move all the libs importing to the entry bin script #3418

Closed
chawyehsu opened this issue Apr 30, 2019 · 1 comment · Fixed by #4896
Closed

move all the libs importing to the entry bin script #3418

chawyehsu opened this issue Apr 30, 2019 · 1 comment · Fixed by #4896

Comments

@chawyehsu
Copy link
Member

chawyehsu commented Apr 30, 2019

I found that the mode of importing lib scripts from different libexec/lib scripts causes a really bad issue - some of the libs are imported and executed many times. This can cause other potential problems and effect performance probably.

CASE

  1. In the bin\scoop.ps1 entry bin script, it imported lib\core.ps1 and lib\bucket.ps1::

https://github.com/lukesampson/scoop/blob/2db651c13a83f1c0ebb53723716efb5d96018fc0/bin/scoop.ps1#L7-L9

  1. In the libexec\scoop-bucket.ps1 script, it imported lib\core.ps1 and lib\bucket.ps1:

https://github.com/lukesampson/scoop/blob/2db651c13a83f1c0ebb53723716efb5d96018fc0/libexec/scoop-bucket.ps1#L22-L23

  1. And in the lib\bucket.ps1 script, it also imported lib\core.ps1:

https://github.com/lukesampson/scoop/blob/2db651c13a83f1c0ebb53723716efb5d96018fc0/lib/buckets.ps1#L1

So, when I run scoop bucket list, the core lib lib\core.ps1 is actually imported by FOUR times, and scoop will execute all of the code of lib\core.ps1 by four times. You can simply add a line of code to verify it:

diff --git a/lib/core.ps1 b/lib/core.ps1
index 148f12539..6e6c54df9 100644
--- a/lib/core.ps1
+++ b/lib/core.ps1
@@ -10,6 +10,8 @@ if((test-path $oldscoopdir) -and !$env:SCOOP) {
     $scoopdir = $oldscoopdir
 }
 
+Write-Host "Hi"
+
 $globaldir = $env:SCOOP_GLOBAL, "$env:ProgramData\scoop" | Select-Object -first 1
 
 # Note: Setting the SCOOP_CACHE environment variable to use a shared directory

Then you will see 4 line of Hi printed before the bucket list.

$ scoop bucket list  
Hi
Hi
Hi
Hi
extras
main

Holy crap! And you will see all other libexec scripts are suffering this issue.

To be honest, I have realized this problem of scoop before I started to maintain pshazz. I have released a patch for pshazz, which was also effected, by changing to import all libs from the pshazz.ps1 entry script.

I didn't make patches for scoop because of the main bucket extraction event. And now I point out the issue to see if anyone has interest to fix this.

@Ash258
Copy link
Contributor

Ash258 commented May 11, 2019

Best solution for this.
We should keep that current behaviour when each module is usable standalone. So you could easily import lib\manifest.ps1 in some external script and everything will work fine (no need to manually import all other stuff which is required for manifest module functioning (core and autoupdate in manifest file example))

I would suggest to adopt Powershell modules for this. (Relates #2733)

Demo

# main.ps1
Import-Module "$PSScriptRoot\COSI.psm1" -Force
. "$PSScriptRoot\A.ps1"

Write-Host 'Should see imported message once'
# COSI.psm1
Write-Host 'MODULE IMPORTED' -f Yellow
# A.ps1
Import-Module "$PSScriptRoot\COSI.psm1"

Write-host 'ANO' -f Yellow

Execute main and you will see only 1 message about imports (unless you specify Force parameter in A file)

Result

image

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 a pull request may close this issue.

3 participants