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

vars are not replaced when used in files[].src #3193

Closed
mpal9000 opened this issue Oct 24, 2024 · 13 comments · Fixed by #3194
Closed

vars are not replaced when used in files[].src #3193

mpal9000 opened this issue Oct 24, 2024 · 13 comments · Fixed by #3194
Labels
bug Something isn't working

Comments

@mpal9000
Copy link

mpal9000 commented Oct 24, 2024

aqua info

$ aqua info
{
  "version": "2.36.1",
  "commit_hash": "423bf97060599e911f9a5a2c5622cf886673dd65",
  "os": "linux",
  "arch": "amd64",
  "pwd": "/home/(USER)/tmp/aqua-test/a",
  "root_dir": "/home/(USER)/.local/share/aqua",
  "env": {
    "AQUA_DISABLE_LAZY_INSTALL": "true",
    "AQUA_GLOBAL_CONFIG": "/home/(USER)/.config/aqua/aqua.yaml",
    "AQUA_POLICY_CONFIG": "/home/(USER)/.config/aqua/policy.yaml",
    "AQUA_ROOT_DIR": "/home/(USER)/.local/share/aqua"
  },
  "config_files": [
    {
      "path": "/home/(USER)/tmp/aqua-test/a/aqua.yaml"
    }
  ]
}

Overview

Custom template variables (vars) are not replaced when used in files[].src.

How to reproduce

aqua.yaml

---
checksum:
  supported_envs:
    - linux
registries:
  - type: local
    name: local
    path: registry.yaml
packages:
  - name: ziglang/zig@nightly
    registry: local
    vars:
      version: 0.14.0-dev.1954+2d888a8e6

Other related code such as local Registry

registry.yaml

---
packages:
  - type: http
    repo_owner: ziglang
    repo_name: zig
    vars:
      - name: version
        required: true
    supported_envs:
      - linux/amd64
      - linux/arm64
    replacements:
      amd64: x86_64
    format: tar.xz
    url: https://ziglang.org/builds/zig-{{.OS}}-{{.Arch}}-{{.Vars.version}}
    files:
      - name: zig
        src: zig-{{.OS}}-{{.Arch}}-{{.Vars.version}}/{{.FileName}} # error
        # src: zig-{{.OS}}-{{.Arch}}-0.14.0-dev.1954+2d888a8e6/{{.FileName}} # ok

Executed command and output

$ aqua install zig
INFO[0000] create a symbolic link                        aqua_version=2.36.1 command=zig env=linux/amd64 program=aqua
INFO[0000] download and unarchive the package            aqua_version=2.36.1 env=linux/amd64 package_name=ziglang/zig package_version=nightly program=aqua registry=local
ERRO[0006] check file_src is correct                     aqua_version=2.36.1 env=linux/amd64 error="check file_src is correct: exe_path isn't found: stat /home/(USER)/.local/share/aqua/pkgs/http/ziglang.org/builds/zig-linux-x86_64-0.14.0-dev.1954+2d888a8e6.tar.xz/zig-linux-x86_64-<no value>/zig: no such file or directory" exe_path="/home/(USER)/.local/share/aqua/pkgs/http/ziglang.org/builds/zig-linux-x86_64-0.14.0-dev.1954+2d888a8e6.tar.xz/zig-linux-x86_64-<no value>/zig" file_name=zig package_name=ziglang/zig package_version=nightly program=aqua registry=local
ERRO[0006] executable files aren't found
Files in the unarchived package (Only 30 files are shown):
zig-linux-x86_64-0.14.0-dev.1954+2d888a8e6/doc/langref.html
zig-linux-x86_64-0.14.0-dev.1954+2d888a8e6/lib/c.zig
# ...
   aqua_version=2.36.1 env=linux/amd64 package_name=ziglang/zig package_version=nightly program=aqua registry=local
ERRO[0006] install the package                           aqua_version=2.36.1 env=linux/amd64 error="check file_src is correct" package_name=ziglang/zig package_version=nightly program=aqua registry=local
FATA[0006] aqua failed                                   aqua_version=2.36.1 env=linux/amd64 error="it failed to install some packages" program=aqua

Expected behaviour

Expected files[].src in

files:
  - name: zig
    src: zig-{{.OS}}-{{.Arch}}-{{.Vars.version}}/{{.FileName}}

when vars: version: 0.14.0-dev.1954+2d888a8e6, to become

zig-linux-x86_64-0.14.0-dev.1954+2d888a8e6/zig

Actual behaviour

It becomes

zig-linux-x86_64-<no value>/zig
@mpal9000 mpal9000 added the bug Something isn't working label Oct 24, 2024
@suzuki-shunsuke
Copy link
Member

Thank you for your feedback.
I'll take a look.
But I don't think you have to use vars.

How about this?

registry.yaml:

---
packages:
  - type: http
    repo_owner: ziglang
    repo_name: zig
    supported_envs:
      - linux
    replacements:
      amd64: x86_64
    format: tar.xz
    url: https://ziglang.org/builds/zig-{{.OS}}-{{.Arch}}-{{.Version}}
    files:
      - name: zig
        src: zig-{{.OS}}-{{.Arch}}-{{.Version}}/{{.FileName}}

aqua.yaml:

---
checksum:
  supported_envs:
    - linux
registries:
  - type: local
    name: local
    path: registry.yaml
packages:
  - name: ziglang/zig@0.14.0-dev.1954+2d888a8e6
    registry: local

@suzuki-shunsuke
Copy link
Member

I'll update aqua-registry for zig's dev versions.

@suzuki-shunsuke
Copy link
Member

v4.239.1 is out 🎉
https://github.com/aquaproj/aqua-registry/releases/tag/v4.239.1

aqua.yaml:

registries:
  - type: standard
    ref: v4.239.1 # renovate: depName=aquaproj/aqua-registry
packages:
  - name: ziglang/zig@0.14.0-dev.1954+2d888a8e6

@suzuki-shunsuke
Copy link
Member

This issue would be solved by #3194

@suzuki-shunsuke
Copy link
Member

@mpal9000
Copy link
Author

mpal9000 commented Oct 24, 2024

Thanks for the quick response, the fix in #3194 and the alternative solution using contains.

  1. Regarding the alternative solution with the version as a variable I was planning to replace the variable's value programmatically where @nightly is declared. Since I can't handle (select, update, remove, etc.) such "nightly" versions automatically in a built-in way. I opened Enhance version fetching and support nightly versions #3198 for a possible solution on fetching the "nightly" and "release" versions.

  2. Regarding variable replacement, I see I can't use them also in minisign.public_key. I was looking for a way to define the key once, using the default value of a variable and avoid its duplication in "overrides". Although in some cases it would be preferable to not be able to override the default value via aqua.yaml.

    a. Is there a documentation page about where the usage of variables is valid?
    b. What about supporting variables that cannot be overridden? Using a new attribute under vars.

  3. Regarding the zig recipe you updated I cannot use it. I think because you need to apply trimV on the non-nightly version paths. I.e.:

  • url: https://ziglang.org/download/{{trimV .Version}}/zig-{{.OS}}-{{.Arch}}-{{trimV .Version}}.{{.Format}}

  • src: zig-{{.OS}}-{{.Arch}}-{{trimV .Version}}/zig

    Also minisign can be used there (I see feat(ziglang/zig): support minisign aqua-registry#24877).

    Edit: Actually I was declaring the version in aqua.yaml with the v prefix. But I'm thinking that both ways (with or without the prefix) are documented (e.g. https://aquaproj.github.io/docs/#introduction) and that regardless of what the tool is using to tag the release. In order to avoid confusion and typos, maybe it would be better to always allow both ways to declare a version in aqua.yaml and CLI (for any tool) and handle that in all registry.yaml files as required, in order to declare the correct paths. For example in the mentioned zig registry file, using trimV would allow to declare the versions in aqua.yaml, with or without the v prefix.

@mpal9000
Copy link
Author

mpal9000 commented Oct 24, 2024

also In the zig recipe the version_source: github_tag attribute could be removed in order to use the github releases, that allows to display the release comments when running aqua g -s. Although it doesn't provide some older releases, so not sure what is your preference.

@suzuki-shunsuke
Copy link
Member

Also minisign can be used there (I see aquaproj/aqua-registry#24877).

I forgot aquaproj/aqua-registry#24877 .
aquaproj/aqua-registry#24877 was conflicted, so I created and merged a new pr.

Regarding variable replacement, I see I can't use them also in minisign.public_key.

For now templates can't be used in minisign.public_key.
If there are needs for it, we'll consider it.

Is there a documentation page about where the usage of variables is valid?

We're sorry but there is no document about it.
At least, I think vars is available in the following fields.

  • asset
  • url
  • files[].src
  • files[].dir
  • checksum.asset
  • checksum.url

In order to avoid confusion and typos, maybe it would be better to always allow both ways to declare a version in aqua.yaml and CLI (for any tool) and handle that in all registry.yaml files as required, in order to declare the correct paths.

That's a good point.
I can see your opinion, but the current basic rule is that each tool version should be GitHub releases or tags and we don't modify them basically.

also In the zig recipe the version_source: github_tag attribute could be removed in order to use the github releases, that allows to display the release comments when running aqua g -s. Although it doesn't provide some older releases, so not sure what is your preference.

When we added the package zig at 2022-04-05, all GitHub Releases are pre-release so we needed to use version_source: github_tag.

Now there are some releases, so we can remove version_source: github_tag.

As you mentioned there are pros and cons.

  • version_source: github_tag allows us to choose old versions
  • removing version_source: github_tag allows us to see release notes

I don't have any strong opinion about this.

@mpal9000
Copy link
Author

mpal9000 commented Oct 24, 2024

Thanks for #28121.

For now templates can't be used in minisign.public_key. If there are needs for it, we'll consider it.

The use case is to avoid the public_key's duplication under "overrides" (that could be solved with a variable + 2.b above).

We're sorry but there is no document about it.

No problem the list you wrote above and the source will be enough for me.

I can see your opinion, but the current basic rule is that each tool version should be GitHub releases or tags and we don't modify them basically.

The "registry.yaml" file can be authored/maintained by a few people (or a single person). On the other hand the "aqua.yaml" files can be authored by many, under many project directories. And those many people have to remember that tool A needs "@x-v1.2.3", tool B needs "@1.2.3", tool C needs "@v1.2.3", etc. I just see that it may lead to confusion more often than expected and it could be solved for most tools, if all those differences would be always handled in "registry.yaml" files (which also change less frequently). And allow authors of "aqua.yaml" files, to declare a tool's version in more than one ways (when possible). Not something blocking for me though. Just sharing my thoughts after a day of using this nice tool.

I don't have any strong opinion about this.

Me neither. I would choose the releases as a source, because of the extra info that can be displayed when searching and because the missing tag-only versions are not a lot (+ all versions are still < 1.0.0).

@suzuki-shunsuke
Copy link
Member

suzuki-shunsuke commented Oct 24, 2024

The use case is to avoid the public_key's duplication under "overrides" (that could be solved with a variable + 2.b above).

I see. It makes sense though we can't prioritize it.
The workaround is to use YAML anchors.

The "registry.yaml" file can be authored/maintained by a few people (or a single person). On the other hand the "aqua.yaml" files can be authored by many, under many project directories. And those many people have to remember that tool A needs "@x-v1.2.3", tool B needs "@1.2.3", tool C needs "@v1.2.3", etc. I just see that it may lead to confusion more often that needed and it could be solved for most tools, if all those differences would be always handled in "registry.yaml" files (which also change less frequently). And allow authors of "aqua.yaml" files, to declare a tool's version in more than one ways (when possible). Not something blocking for me though. Just sharing my thoughts after a day of using this nice tool.

Thank you for sharing your valuable feedback.
We don't assume users edit versions in aqua.yaml by hand often because they can update aqua.yaml by aqua g -i [-s], aqua up [-s], and Renovate.
But I also think some users would edit versions by hand as they don't know these commands.

Edit:

We don't assume users edit versions in aqua.yaml by hand often because they can update aqua.yaml by aqua g -i [-s], aqua up [-s], and Renovate.

zig's dev versions are one of edge cases. 😅

@mpal9000
Copy link
Author

The workaround is to use YAML anchors.

Thanks, tried with anchors and it works. Although I get an LSP error on the editor, from the yaml schema (for the new property I'm using as a parent of the anchor). But it's ok for now.

We don't assume users edit versions in aqua.yaml by hand often because they can update aqua.yaml by aqua g -i [-s], aqua up [-s], and Renovate. But I also think some users would edit versions by hand as they don't know these commands.

I see, although I think it would be nice to support the manual edit workflow at some point, since it is frequently used in similar tools. And the declarative nature of aqua, also means that the file that you are declaring things, is easily editable. Also you can do more with the manual editing currently. For example you can not declare a tool version using the version attribute, when using aqua g -i -s (in order to block the automatic update for that version). So I could run aqua g -i -s select two versions of a tool, then run aqua up -i and end up with a duplicated declaration of the same "name@version".

zig's dev versions are one of edge cases.

Hope not for long #3198, #682 :-)

@suzuki-shunsuke
Copy link
Member

For example you can not declare a tool version using the version attribute, when using aqua g -i -s (in order to block the automatic update for that version). So I could run aqua g -i -s select two versions of a tool, then run aqua up -i and end up with a duplicated declaration of the same "name@version".

You can use -pin option.

$ aqua g -pin cli/cli
- name: cli/cli
  version: v2.60.0

@mpal9000
Copy link
Author

Nice! Wrong example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants