Skip to content

yqutil: fix to preserve line breaks #2625

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,10 @@ jobs:
run: make
- name: Install
run: sudo make install
- name: Verify templates match `limactl edit` format
run: |
find examples -name '*.yaml' -exec limactl edit --set 'del(.nothing)' {} \;
git diff-index --exit-code HEAD
- name: Uninstall
run: sudo make uninstall

Expand Down
10 changes: 5 additions & 5 deletions examples/buildkit.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@
# $ export BUILDKIT_HOST=$(limactl list buildkit --format 'unix://{{.Dir}}/sock/buildkitd.sock')
# $ buildctl debug workers
message: |
To run `buildkit` on the host (assumes buildctl is installed), run the following commands:
-------
export BUILDKIT_HOST="unix://{{.Dir}}/sock/buildkitd.sock"
buildctl debug workers
-------
To run `buildkit` on the host (assumes buildctl is installed), run the following commands:
-------
export BUILDKIT_HOST="unix://{{.Dir}}/sock/buildkitd.sock"
buildctl debug workers
-------
images:
# Try to use release-yyyyMMdd image if available. Note that release-yyyyMMdd will be removed after several months.
- location: "https://cloud-images.ubuntu.com/releases/24.04/release-20240821/ubuntu-24.04-server-cloudimg-amd64.img"
Expand Down
12 changes: 6 additions & 6 deletions examples/default.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -264,10 +264,10 @@ containerd:
# Setting of instructions is supported like this: "qemu64,+ssse3".
# 🟢 Builtin default: hard-coded arch map with type (see the output of `limactl info | jq .defaultTemplate.cpuType`)
cpuType:
# aarch64: "cortex-a72" # (or "host" when running on aarch64 host)
# armv7l: "cortex-a7" # (or "host" when running on armv7l host)
# riscv64: "rv64" # (or "host" when running on riscv64 host)
# x86_64: "qemu64" # (or "host,-pdpe1gb" when running on x86_64 host)
# aarch64: "cortex-a72" # (or "host" when running on aarch64 host)
# armv7l: "cortex-a7" # (or "host" when running on armv7l host)
# riscv64: "rv64" # (or "host" when running on riscv64 host)
# x86_64: "qemu64" # (or "host,-pdpe1gb" when running on x86_64 host)

rosetta:
# Enable Rosetta for Linux (EXPERIMENTAL; will graduate from experimental in Lima v1.0).
Expand Down Expand Up @@ -456,8 +456,8 @@ hostResolver:
# predefined to specify the gateway address to the host.
# 🟢 Builtin default: null
hosts:
# guest.name: 127.1.1.1
# host.name: host.lima.internal
# guest.name: 127.1.1.1
# host.name: host.lima.internal

# If hostResolver.enabled is false, then the following rules apply for configuring dns:
# Explicitly set DNS addresses for qemu user-mode networking. By default qemu picks *one*
Expand Down
36 changes: 28 additions & 8 deletions pkg/yqutil/yqutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"os"
"strings"

"github.com/google/yamlfmt"
"github.com/google/yamlfmt/formatters/basic"
"github.com/mikefarah/yq/v4/pkg/yqlib"
"github.com/sirupsen/logrus"
Expand All @@ -15,13 +16,26 @@ import (
// EvaluateExpression evaluates the yq expression, and returns the modified yaml.
func EvaluateExpression(expression string, content []byte) ([]byte, error) {
logrus.Debugf("Evaluating yq expression: %q", expression)
formatter, err := yamlfmtBasicFormatter()
if err != nil {
return nil, err
}
// `ApplyFeatures()` is being called directly before passing content to `yqlib`.
// This results in `ApplyFeatures()` being called twice with `FeatureApplyBefore`:
// once here and once inside `formatter.Format`.
// Currently, calling `ApplyFeatures()` with `FeatureApplyBefore` twice is not an issue,
// but future changes to `yamlfmt` might cause problems if it is called twice.
contentModified, err := formatter.Features.ApplyFeatures(content, yamlfmt.FeatureApplyBefore)
if err != nil {
return nil, err
}
tmpYAMLFile, err := os.CreateTemp("", "lima-yq-*.yaml")
if err != nil {
return nil, err
}
tmpYAMLPath := tmpYAMLFile.Name()
defer os.RemoveAll(tmpYAMLPath)
Copy link
Member

Choose a reason for hiding this comment

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

I know this is not part of this PR, but why is this calling os.RemoveAll() instead of os.Remove() given that tmpYAMLPath is a file and not a directory.

@afbjorklund Do you remember why you choose RemoveAll?

_, err = tmpYAMLFile.Write(content)
_, err = tmpYAMLFile.Write(contentModified)
if err != nil {
tmpYAMLFile.Close()
return nil, err
Expand Down Expand Up @@ -70,7 +84,7 @@ func EvaluateExpression(expression string, content []byte) ([]byte, error) {
return nil, err
}

return yamlfmt(out.Bytes())
return formatter.Format(out.Bytes())
}

func Join(yqExprs []string) string {
Expand All @@ -80,17 +94,23 @@ func Join(yqExprs []string) string {
return strings.Join(yqExprs, " | ")
}

func yamlfmt(content []byte) ([]byte, error) {
func yamlfmtBasicFormatter() (*basic.BasicFormatter, error) {
factory := basic.BasicFormatterFactory{}
config := map[string]interface{}{
"indentless_arrays": true,
"line_ending": "lf", // prefer LF even on Windows
"pad_line_comments": 2,
"retain_line_breaks": true, // does not affect to the output because yq removes empty lines before formatting
"indentless_arrays": true,
"line_ending": "lf", // prefer LF even on Windows
"pad_line_comments": 2,
"retain_line_breaks": true,
"retain_line_breaks_single": false,
}

formatter, err := factory.NewFormatter(config)
if err != nil {
return nil, err
}
return formatter.Format(content)
basicFormatter, ok := formatter.(*basic.BasicFormatter)
if !ok {
return nil, fmt.Errorf("unexpected formatter type: %T", formatter)
}
return basicFormatter, nil
}
1 change: 1 addition & 0 deletions pkg/yqutil/yqutil_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ memory: null
expected := `
# CPUs
cpus: 2

# Memory size
memory: 2GiB
`
Expand Down
Loading