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

Variable for package manager #5436

Merged
merged 4 commits into from
Mar 10, 2023
Merged

Conversation

rjbou
Copy link
Collaborator

@rjbou rjbou commented Feb 6, 2023

Add global config field sys-pkg-manager-cmd to be able to specify for MSYS2 & Cygwin the command path. In can be used later to specify more precisely used package manager when several exists in the machine, see. #4440
It also permit to not rely on defined variables with the full integration in config file. cc @jonahbeckford
related to #5348

@rjbou rjbou added PR: QUEUED Pending pull request, waiting for other work to be merged or closed PR: WIP Not for merge at this stage labels Feb 6, 2023
@rjbou rjbou added this to the 2.2.0~alpha milestone Feb 6, 2023
@rjbou rjbou force-pushed the variable-for-package-manager branch from 9e198f1 to d79979a Compare February 7, 2023 15:37
@rjbou rjbou removed the PR: WIP Not for merge at this stage label Feb 7, 2023
@rjbou rjbou requested a review from dra27 February 7, 2023 15:43
@dra27 dra27 force-pushed the variable-for-package-manager branch from d79979a to be61d81 Compare February 8, 2023 17:10
Copy link
Member

@dra27 dra27 left a comment

Choose a reason for hiding this comment

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

LGTM, and ready to go when the underlying PRs it's based on are done.

For Diskuv, I think it means that:

run_opam var --global "sys-pkg-manager-cmd-msys2=$syspkgmgrpath"

becomes:

run_opam option --global "sys-pkg-manager-cmd+=[\"msys2\" \"$syspkgmgrpath"\"]"

@rjbou
Copy link
Collaborator Author

rjbou commented Feb 8, 2023

it would be:

run_opam option --global "sys-pkg-manager-cmd+=[\"msys2\" \"$syspkgmgrpath"\"]"

@dra27
Copy link
Member

dra27 commented Feb 8, 2023

Oops - poor copy and pasting from the test on my part!

@jonahbeckford
Copy link
Contributor

jonahbeckford commented Feb 9, 2023

Thanks for the translation! sys-pkg-manager-cmd+=[\"msys2\" \"$syspkgmgrpath"\"] is not idempotent though. I'll figure out something to mitigate.

[Jonah] There is some new behavior in += that I don't understand. It is idempotent ... meaning that msys2 <path> will always be a single, latest entry regardless of how often += has been used. That doesn't make sense mathematically, but the new behavior is welcome!

This is big enough that I should actually test this PR. I just don't have the bandwidth this week; can this wait until next week?

@jonahbeckford
Copy link
Contributor

jonahbeckford commented Feb 16, 2023

Mostly the testing went fine, but it aborted with exit code 1 at the last step:

beckf@DKMCHO-WINDSK-A CLANG64 /z/source/dkml-component-opam
$ opam install ./depexts-test/conf-pkg-config.2 --verbose --debug-level 2
00:02.425  CLIENT                 INSTALL conf-pkg-config
00:02.880  SYSTEM                 [log-23564-951126] (in 0.395s) ocamlc -vnum
00:03.201  SYSTEM                 mkdir C:\Users\beckf\AppData\Local\Temp\opam-23564-4defcc
00:03.438  PROC                   cygvoke(glob): "C:\Users\beckf\AppData\Local\Programs\DiskuvOCaml\tools\MSYS2\usr\bin\pacman.exe" "-Si"
00:05.897  SYSTEM                 rmdir C:\Users\beckf\AppData\Local\Temp\opam-23564-4defcc
00:07.969  XSYS                   Adding to env { LC_ALL=C }
00:07.989  PROC                   cygvoke(glob): "C:\Users\beckf\AppData\Local\Programs\DiskuvOCaml\tools\MSYS2\usr\bin\pacman.exe" "-Qs" "^(patchutils|mingw-w64-clang-x86_64-pkgconf|mingw-w64-clang-x86_64-pkg-config|graphviz|diffutils|binutils)$"
00:09.483  STATE                  depexts loaded in 6.281s
00:09.493  STATE                  Availability of packages computed in 7.068s.
00:09.519  STATE                  Detected changed packages (marked for reinstall): {}
00:09.520  FILE(package-version-list)  Cannot find Z:\source\dkml-component-opam\opamroot\default\.opam-switch\reinstall
00:09.966  STATE                  Universe load: 0.393s
00:09.966  SOLVER                 resolve request=install:conf-pkg-config (= 2) remove:() upgrade:()
00:12.160  SOLVER                 Load cudf universe (depopts:false, build:true, post:true)
00:12.160  SOLVER                 Load cudf universe (depopts:false, build:true, post:true)
00:12.364  CUDF                   resolve request=install:conf-pkg-config (= 5) remove:() upgrade:()
00:12.364  CUDF                   Checking request...
00:15.473  CUDF                   Request checked in 3.109s
00:15.639  CUDF                   Conflicts: 34 (0) pkgs to remove
00:15.639  CUDF                   Preprocess cudf request (trimming: full): from 25470 to 962 packages in 0.17s
00:15.639  SOLVER                 Calling solver builtin-mccs+glpk with criteria -removed,-count[avoid-version,changed],-count[version-lag,request],-count[version-lag,changed],-count[missing-depexts,changed],-changed
Can reduce graph.
Initial size: 962 packages (8 installed, 954 uninstalled), 74 virtual packages
Final size: 962 packages (8 installed, 954 uninstalled), 54 virtual packages
Constructing initial basis...
Number of 0-1 knapsack inequalities = 2090
Constructing conflict graph...
Conflict graph has 713 + 106 = 819 vertices
00:17.031  SOLVER                 External solver took 1.379s
00:17.031  CUDF                   Solver call done in 1.558s
00:17.031  SOLVER                 Load cudf universe (depopts:true, build:false, post:false)
00:17.031  SOLVER                 Load cudf universe (depopts:true, build:false, post:false)
00:17.205  SOLVER                 Load cudf universe (depopts:true, build:true, post:false)
00:17.205  SOLVER                 Load cudf universe (depopts:true, build:true, post:false)
00:17.414  CUDF                   graph_of_actions root_actions={  - ✶ conf-pkg-config (= 5) }
00:17.447  CUDF                   Removed unrelated actions: {}
00:17.447  SOLUTION               apply
The following actions will be performed:
=== install 1 package
  ✶ conf-pkg-config 2 (pinned)
00:17.464  SYSTEM                 mkdir C:\Users\beckf\AppData\Local\Temp\opam-23564-8136bf
00:17.474  PROC                   cygvoke(glob): "C:\Users\beckf\AppData\Local\Programs\DiskuvOCaml\tools\MSYS2\usr\bin\pacman.exe" "-Si"
00:20.025  SYSTEM                 rmdir C:\Users\beckf\AppData\Local\Temp\opam-23564-8136bf
00:22.095  XSYS                   Adding to env { LC_ALL=C }
00:22.116  PROC                   cygvoke(glob): "C:\Users\beckf\AppData\Local\Programs\DiskuvOCaml\tools\MSYS2\usr\bin\pacman.exe" "-Qs" "^(mingw-w64-clang-x86_64-pkgconf|mingw-w64-clang-x86_64-pkg-config)$"
00:23.611  STATE                  depexts loaded in 6.147s

The following system packages will first need to be installed:
    mingw-w64-clang-x86_64-pkg-config

<><> Handling external dependencies <><><><><><><><><><><><><><><><><><><><><><>

opam believes some required external dependencies are missing. opam can:
> 1. Run C:\Users\beckf\AppData\Local\Programs\DiskuvOCaml\tools\MSYS2\usr\bin\pacman.exe to install them (may need root/sudo access)
  2. Display the recommended C:\Users\beckf\AppData\Local\Programs\DiskuvOCaml\tools\MSYS2\usr\bin\pacman.exe command and wait while you run it manually (e.g. in another terminal)
  3. Attempt installation anyway, and permanently register that this external dependency is present, but not detectable
  4. Abort the installation

[1/2/3/4]

01:43.656  PROC                   cygvoke(glob): "C:\Users\beckf\AppData\Local\Programs\DiskuvOCaml\tools\MSYS2\usr\bin\pacman.exe" "-Su" "--noconfirm" "mingw-w64-clang-x86_64-pkg-config"  
+ C:\Users\beckf\AppData\Local\Programs\DiskuvOCaml\tools\MSYS2\usr\bin\pacman.exe "-Su" "--noconfirm" "mingw-w64-clang-x86_64-pkg-config"

PS Z:\source\dkml-component-opam> echo $LASTEXITCODE
1

I'll see if it is obvious (soon, not now) what is going wrong on the last step.

@jonahbeckford
Copy link
Contributor

Err. I ran it again and it worked. Hopefully that was just something silly like the anti-virus kicking in.

Thanks for the PR! Works great.

jonahbeckford added a commit to diskuv/dkml-runtime-distribution that referenced this pull request Feb 16, 2023
@rjbou rjbou force-pushed the variable-for-package-manager branch from be61d81 to f35d997 Compare March 1, 2023 12:27
@dra27
Copy link
Member

dra27 commented Mar 2, 2023

This still seems to have the commits from #4926 in it?

@rjbou rjbou force-pushed the variable-for-package-manager branch from f35d997 to 82d13bc Compare March 6, 2023 14:29
@rjbou rjbou removed the PR: QUEUED Pending pull request, waiting for other work to be merged or closed label Mar 7, 2023
@dra27 dra27 merged commit c090675 into ocaml:master Mar 10, 2023
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 this pull request may close these issues.

3 participants