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

Read/write issues on package config variables #4489

Open
LasseBlaauwbroek opened this issue Jan 2, 2021 · 6 comments · Fixed by #4903
Open

Read/write issues on package config variables #4489

LasseBlaauwbroek opened this issue Jan 2, 2021 · 6 comments · Fixed by #4903

Comments

@LasseBlaauwbroek
Copy link
Contributor

LasseBlaauwbroek commented Jan 2, 2021

The following is tested on Opam 2.1.0~beta2: Create an opam package my-package, with the following config file:

opam-version: "2.0"
variables {
          myvar: ["hello" "hello world"]
}
  • [Read issue] Running opam var my-package:myvar results in hello hello world. This is problematic because I can not distinguish separate list items from spaces in a string. One possible solution would be to return the array verbatim, or at least retain the quotes.
  • [Write issue] The help for opam var advertises the following syntax:
opam var [OPTION]... [VAR[=[VALUE]]]

However, running opam var my-package:myvar=test returns an error:

[ERROR] Variable my-package:myvar=test not found in in switch

(Note also the double "in" in the error message)

  • [Feature request] In addition, it would be useful if opam var supports += and -= like opam option does.
@LasseBlaauwbroek
Copy link
Contributor Author

Additional feature request: It would be nice if configuration variables could contain variables themselves. These should then be resolved when being read.

@LasseBlaauwbroek
Copy link
Contributor Author

Additional bug: During installation, options with an empty array myvar: [] are filtered out. Then running opam var my-package:myvar gives an error. However, if I manually modify the installed my-package.config file, and re-add the variable, I do not get an error from opam var my-package:myvar.

@rjbou rjbou added this to To do in Opam 2.1.x via automation Jan 5, 2021
@rjbou rjbou added this to the 2.1.0~beta5 milestone Jan 5, 2021
@rjbou rjbou added this to To do in Feature Wish via automation Jan 5, 2021
@rjbou rjbou added the AREA: UI label Jan 5, 2021
@rjbou
Copy link
Collaborator

rjbou commented Jan 5, 2021

opam var documentation is not clear on package variables. This commands is designed to act (read/write) on opam defined variables : global and per switch ; and to read packages variable; It can't modify package defined variables as for that it's a package (opam or config file) modification.

For append, as opam variables can be string, string list or bool, it doesn't suit for such operation.

On variables display, we could display the same way than opam variables, ie verbatim. You'll have i this case ["hello" "hello world"] as my-var is a list.

@LasseBlaauwbroek
Copy link
Contributor Author

It can't modify package defined variables as for that it's a package (opam or config file) modification.

Opam copies the config files of packages to <prefix>/.opam-switch/config/<pkg-name>.config. It seems to me that opam var should be able to safely modify this file. The modification would not change the package, but just the local config file of that package. The manual does not make this 100% clear, but I cannot find anywhere that this config file is meant to be immutable.

For append, as opam variables can be string, string list or bool, it doesn't suit for such operation.

In case of a string list, it would be a suitable operation right?

On variables display, we could display the same way than opam variables, ie verbatim. You'll have i this case ["hello" "hello world"] as my-var is a list.

That's what I'd propose as well.

@dra27 dra27 removed this from To do in Opam 2.1.x Jan 22, 2021
@dra27 dra27 modified the milestones: 2.1.0~beta5, 2.2.0~alpha Jan 22, 2021
@dra27 dra27 added this to To do in Opam 2.2.0 via automation Jun 25, 2021
@dra27 dra27 removed this from To do in Feature Wish Jun 25, 2021
@dra27
Copy link
Member

dra27 commented Jun 25, 2021

Updates from a developer meeting this morning: control of package variables is a likely feature for opam 2.3. In the meantime, there are two changes which we will slate for 2.2:

  • The documentation should be explicit that package variables are read-only
  • The syntax opam var my-package:myvar=test should be recognised by opam in order to display a more meaningful error. Depending on whether my-package is installed, opam should display my-package is not installed (package variables are also read-only) or Package variables are read-only and cannot be updated using `opam var`.

@rjbou rjbou self-assigned this Nov 8, 2021
@rjbou rjbou moved this from To do to In progress in Opam 2.2.0 Nov 16, 2021
@rjbou rjbou modified the milestones: 2.2.0~alpha, 2.3 Jun 7, 2023
@rjbou rjbou added this to To do in Opam 2.3 via automation Jun 7, 2023
@rjbou rjbou moved this from In progress to After ~alpha; before release in Opam 2.2.0 Jun 12, 2023
Opam 2.2.0 automation moved this from After ~alpha; before release to Done Jul 12, 2023
Opam 2.3 automation moved this from To do to Done Jul 12, 2023
@rjbou rjbou reopened this Jul 24, 2023
Opam 2.2.0 automation moved this from Done to In progress Jul 24, 2023
@rjbou
Copy link
Collaborator

rjbou commented Jul 24, 2023

Documentation still need to be updated

@rjbou rjbou removed this from In progress in Opam 2.2.0 Jul 24, 2023
@rjbou rjbou removed this from the 2.3 milestone Jul 24, 2023
@rjbou rjbou added this to the 2.2.0~alpha2 milestone Jul 24, 2023
@rjbou rjbou moved this from Done to To do in Opam 2.3 Sep 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Opam 2.3
  
To do
Development

Successfully merging a pull request may close this issue.

3 participants