add switch to allow SystemVar Path add #24

andrihitz · 2022-08-17T13:40:42Z

Adds a switch option /SystemVar to add it to the SystemVar Path aswell.

naveen521kk · 2022-08-17T14:07:57Z

choco/tools/chocolateyinstall.ps1

+# checks if choco switch var is in parameters
+if ($pp['SystemVar'] -eq 'true') {
+	Write-Host "Running tlmgr SystemVar path add"
+	# Adds to SystemVar Path
+	[Environment]::SetEnvironmentVariable("PATH", $Env:PATH + ";C:\Program Files\Scripts", [EnvironmentVariableTarget]::Machine)
+}


If I understand the code correctly it adds C:\Program Files\Scripts to the system path. Is there any reason why C:\Program Files\Scripts needs to be added to the system path(TinyTex is installed to C:\tools\TinyTeX usually, where C:\tools can be customised)?

By the way, the installation can be added to the system path using the command tlmgr path --w32mode=admin add (I think this should be the default).

On the other hand, SystemVar isn't clear can you rename it to something else like AddToSystemPath instead?

Thanks for your feedback, I accidentally copied the wrong code from my local files. Fixed it.

Does this mean that the installation should be touched again to add it to the system path?

A good point, I just renamed it to match your suggestion. Thanks

cderv

Hi @andrihitz

It is not quite clear to me which issue we are trying to solve here.

tlmgr should be added to PATH using its own mechanism tlmg path add which we do

tinytex-releases/choco/tools/chocolateyinstall.ps1

Lines 16 to 18 in adc16f8

    
           # Adds to Path 
        
           $statementsToRun = "/C `"$toolsDir\TinyTeX\bin\win32\tlmgr.bat path add`"" 
        
           Start-ChocolateyProcessAsAdmin $statementsToRun "cmd.exe"

Is this not enough ? Is it not working ?

as @naveen521kk said, there is some configuration that can be done to this command. It is shown here
https://www.tug.org/texlive/doc/tlmgr.html#path
In some case, the flag is not even needed

If the user has admin rights, and the option --w32mode is not given, the setting w32_multi_user determines the location (i.e., if it is on then the system path, otherwise the user path is changed).

Does chocolatey is executed in Admin mode when you install ?

Anayway, If those configuration needs to be access we can make that happens.

Here we are adding a second way to add to PATH and I am not quite sure if this is a good solution as I don't fully comprehend the issue.

Can you explain what you are solving here ? Thank you.

changed to tlmgr command to add path var to the systemvars

andrihitz · 2022-08-23T14:20:16Z

Hi @cderv

Thanks for your Feedback.

Well, the issue I have, by default, the path var is just installed at the user's path and not in the system path, even if the command prompt was launched as admin.

I misunderstood the note by @naveen521kk for installing the system path with the onboard command in tlmgr.
In the newest commits, I changed my PowerShell command to that command.

For my understanding, I wasn't able to get any information about the w32_multi_user setting and what I had to change to automatically add the system path, Can you explain where I can find this setting?

Thanks for your Support.

cderv · 2022-08-25T12:01:12Z

choco/tools/chocolateyinstall.ps1

+# checks if choco switch AddToSystemPath var is in parameters
+if ($pp['AddToSystemPath'] -eq 'true') {
+   Write-Host "Running tlmgr AddToSystemPath"
+   # AddToSystemPath
+   $statementsToRun = "/C `"$toolsDir\TinyTeX\bin\win32\tlmgr.bat path --w32mode=admin add`""
+   Start-ChocolateyProcessAsAdmin $statementsToRun "cmd.exe"
+}


If we were to do that, I think we just need to add the flag in the previous line when the option is passed to chocolatey.

https://github.com/rstudio/tinytex-releases/pull/24/files#diff-5a71f76f6ddf465237c7306d0e59eb56e11db5cfb7281b4a5a647306dda6efe5R19-R20

Here we are putting tlmgr twice in the PATH. First user, then system. It does not seem right.

combined the two lines, but I am not sure if this is the way to do it.
At least it works with the if operator.

cderv · 2022-08-25T12:12:01Z

For my understanding, I wasn't able to get any information about the w32_multi_user setting and what I had to change to automatically add the system path, Can you explain where I can find this setting?

I think it is a conf for tlmgr. Probably something you can set with tlmgr conf tlmgr w32_multi_user=1 or tinytex::tlmgr_conf(c("tlmgr", "w32_multi_user=1"). Donc for tlmgr conf is in the man page: https://www.tug.org/texlive/doc/tlmgr.html#conf
There are also some config file : https://www.tug.org/texlive/doc/tlmgr.html#CONFIGURATION-FILE-FOR-TLMGR

This is probably what is controlling the default behavior of tlmgr.

Do you want to try one of this configuration file to see if that change something ?

Other we could add the flag, but in the first call of path add and not add a second call IMO

andrihitz · 2022-08-25T12:45:14Z

I guess adding a choco param as we did is the best, quickest solution otherwise, we need to modify or import the config, as I understand this right.

cderv · 2022-08-25T13:20:33Z

The change looks good to me. Did you manage to build and test it yourself ? does it install as admin with this change ?
It looks it should

andrihitz · 2022-08-25T13:25:46Z

Yes, I packed it on my local machine and installed it with and without the parameter, and it worked as intended. The path var will be removed while uninstalling.

choco/tools/chocolateyuninstall.ps1

naveen521kk

small typo, other than that sgtm

README.md

Co-authored-by: Naveen M K <[email protected]>

cderv

Looks good to me too !

@yihui ok to merge ?

Should we trigger a patch release of the chocolatey bundle ? Otherwise it will be available next month.

yihui · 2022-09-08T16:53:24Z

@yihui ok to merge ?

Yes.

Should we trigger a patch release of the chocolatey bundle ? Otherwise it will be available next month.

I don't have an opinion. It's up to @andrihitz (whether a release is needed right now).

cderv · 2022-09-08T18:15:10Z

I have just merged. @andrihitz can you wait for next month automatic release ?

Otherwise, I'll see what I can do for a path release for this version.

andrihitz · 2022-09-13T05:54:39Z

Thanks for merging. We can wait for next month's automatic release. @cderv

andrihitz added 2 commits August 17, 2022 15:39

add switch to allow SystemVar Path add

6d3fbd2

Adds a switch option /SystemVar to add it to the SystemVar Path aswell.

added Parameter to readme

c41a24d

naveen521kk reviewed Aug 17, 2022

View reviewed changes

andrihitz added 2 commits August 17, 2022 16:29

changed var and added right path

3d3da3e

changed variable in readme

fbf41ec

cderv reviewed Aug 22, 2022

View reviewed changes

andrihitz added 2 commits August 23, 2022 13:10

changed to tlmgr command

7cf1c59

changed to tlmgr command to add path var to the systemvars

added remove from systemvars

51d4ca6

cderv reviewed Aug 25, 2022

View reviewed changes

unite the same code

dc6f54a

cderv reviewed Aug 25, 2022

View reviewed changes

choco/tools/chocolateyuninstall.ps1 Outdated Show resolved Hide resolved

check path to remove tlmgr from sysvars

9cd9e86

naveen521kk approved these changes Sep 8, 2022

View reviewed changes

README.md Outdated Show resolved Hide resolved

andrihitz and others added 2 commits September 8, 2022 16:17

Update README.md

11742d1

Co-authored-by: Naveen M K <[email protected]>

Merge branch 'master' into patch-1

4bfbde7

cderv approved these changes Sep 8, 2022

View reviewed changes

cderv merged commit d6d9209 into rstudio:master Sep 8, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add switch to allow SystemVar Path add #24

add switch to allow SystemVar Path add #24

andrihitz commented Aug 17, 2022

naveen521kk Aug 17, 2022

andrihitz Aug 17, 2022

cderv left a comment

andrihitz commented Aug 23, 2022

cderv Aug 25, 2022

andrihitz Aug 25, 2022

cderv commented Aug 25, 2022

andrihitz commented Aug 25, 2022

cderv commented Aug 25, 2022

andrihitz commented Aug 25, 2022

naveen521kk left a comment

cderv left a comment

yihui commented Sep 8, 2022

cderv commented Sep 8, 2022

andrihitz commented Sep 13, 2022

	# Adds to Path
	$statementsToRun = "/C `"$toolsDir\TinyTeX\bin\win32\tlmgr.bat path add`""
	Start-ChocolateyProcessAsAdmin $statementsToRun "cmd.exe"

add switch to allow SystemVar Path add #24

add switch to allow SystemVar Path add #24

Conversation

andrihitz commented Aug 17, 2022

naveen521kk Aug 17, 2022

Choose a reason for hiding this comment

andrihitz Aug 17, 2022

Choose a reason for hiding this comment

cderv left a comment

Choose a reason for hiding this comment

andrihitz commented Aug 23, 2022

cderv Aug 25, 2022

Choose a reason for hiding this comment

andrihitz Aug 25, 2022

Choose a reason for hiding this comment

cderv commented Aug 25, 2022

andrihitz commented Aug 25, 2022

cderv commented Aug 25, 2022

andrihitz commented Aug 25, 2022

naveen521kk left a comment

Choose a reason for hiding this comment

cderv left a comment

Choose a reason for hiding this comment

yihui commented Sep 8, 2022

cderv commented Sep 8, 2022

andrihitz commented Sep 13, 2022