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

wolfictl pmub: just a reverse of wolfictl bump #677

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
27 changes: 21 additions & 6 deletions pkg/cli/bump.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,10 @@ import (
const epochPattern = `epoch: %d`

type bumpOptions struct {
repoDir string
epoch bool
dryRun bool
repoDir string
epoch bool
dryRun bool
increment bool
}

// this feels very hacky but the Makefile is going away with help from Dag so plan to delete this func soon
Expand Down Expand Up @@ -72,6 +73,14 @@ the version in each matching configuration file:
wolfictl bump openssl
wolfictl bump lib*.yaml

wolfictl bump can take a filename, a package or a file glob,
and extra flag increment set to false , decreasing the version in each
matching configuration file:

wolfictl bump --increment=false zlib.yaml
wolfictl bump --increment=false openssl
wolfictl bump --increment=false lib*.yaml

The command assumes it is being run from the top of the wolfi/os
repository. To look for files in another location use the --repo flag.
You can use --dry-run to see which versions will be bumped without
Expand Down Expand Up @@ -120,6 +129,7 @@ modifying anything in the filesystem.
cmd.Flags().BoolVar(&opts.epoch, "epoch", true, "bump the package epoch")
cmd.Flags().BoolVar(&opts.dryRun, "dry-run", false, "don't change anything, just print what would be done")
cmd.Flags().StringVar(&opts.repoDir, "repo", ".", "path to the wolfi/os repository")
cmd.Flags().BoolVar(&opts.increment, "increment", true, "increments/decrements the package epoch")
Copy link
Member

Choose a reason for hiding this comment

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

Would we ever want to bump by anything other than -1 and +1? Would it make sense to have this take an int flag that defaults to 1?

E.g. to bump down, do this:

wolfictl bump --by -1 foo.yaml

If not, then I think I'd prefer this be a negative flag (--decrement or --substract or --minus or --undo) that defaults to false because it's easier to type:

wolfictl bump --undo

Than it is to type:

wolfictl bump --increment=false

But also because I just learned that this would be an error:

wolfictl bump --increment false

(See wolfi-dev/os@bfe82af 🤦)


return cmd
}
Expand All @@ -130,9 +140,14 @@ func bumpEpoch(ctx context.Context, opts bumpOptions, path string) error {
return fmt.Errorf("unable to parse configuration at %q: %w", path, err)
}

newEpoch := cfg.Package.Epoch + 1
if !opts.increment {
newEpoch = cfg.Package.Epoch - 1
}

fmt.Fprintf(
os.Stderr, "bumping %s-%s-%d in %s to epoch %d\n", cfg.Package.Name,
cfg.Package.Version, cfg.Package.Epoch, path, cfg.Package.Epoch+1,
cfg.Package.Version, cfg.Package.Epoch, path, newEpoch,
)

if opts.dryRun {
Expand All @@ -154,7 +169,7 @@ func bumpEpoch(ctx context.Context, opts bumpOptions, path string) error {
found = true
newFile = append(
newFile, strings.ReplaceAll(
scanner.Text(), old, fmt.Sprintf(epochPattern, cfg.Package.Epoch+1),
scanner.Text(), old, fmt.Sprintf(epochPattern, newEpoch),
),
)
} else {
Expand All @@ -173,7 +188,7 @@ func bumpEpoch(ctx context.Context, opts bumpOptions, path string) error {
return fmt.Errorf("writing %s: %w", path, err)
}

if err := updateMakefile(opts.repoDir, cfg.Package.Name, cfg.Package.Version, cfg.Package.Epoch+1); err != nil {
if err := updateMakefile(opts.repoDir, cfg.Package.Name, cfg.Package.Version, newEpoch); err != nil {
return fmt.Errorf("updating makefile: %w", err)
}

Expand Down
100 changes: 100 additions & 0 deletions pkg/cli/bump_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
package cli

import (
"context"
"os"
"path/filepath"
"testing"

"gopkg.in/yaml.v3"
)

func TestBumpEpoch(t *testing.T) {
ctx := context.Background()

// Create a temporary directory for testing
tmpDir, err := os.MkdirTemp("", "test")
if err != nil {
t.Fatalf("Error creating temp directory: %v", err)
}
defer os.RemoveAll(tmpDir)

// Create a temporary config file with a known epoch
configFilePath := filepath.Join(tmpDir, "test_config.yaml")
configContent := `
package:
name: clickhouse
version: 24.2.1.2248
epoch: 6
`
err = os.WriteFile(configFilePath, []byte(configContent), 0o600)
if err != nil {
t.Fatalf("Error creating temp config file: %v", err)
}

tests := []struct {
desc string
increment bool
expectedEpoch int
expectedMessage string
}{
{
desc: "Incrementing epoch",
increment: true,
expectedEpoch: 7,
expectedMessage: "bumping clickhouse-24.2.1.2248-0 in " + configFilePath +
" to epoch 7\n",
},
{
desc: "Decrementing epoch",
increment: false,
expectedEpoch: 6,
expectedMessage: "bumping clickhouse-24.2.1.2248-0 in " + configFilePath +
" to epoch 6\n",
},
{
desc: "Decrementing epoch",
increment: false,
expectedEpoch: 5,
expectedMessage: "bumping clickhouse-24.2.1.2248-0 in " + configFilePath +
" to epoch 5\n",
},
}

for _, tc := range tests {
t.Run(tc.desc, func(t *testing.T) {
opts := bumpOptions{
repoDir: tmpDir,
dryRun: false,
increment: tc.increment,
}

// Test bumping epoch
err := bumpEpoch(ctx, opts, configFilePath)
if err != nil {
t.Fatalf("Error bumping epoch: %v", err)
}

// Read the modified config file to check if the epoch is bumped
modifiedConfigData, err := os.ReadFile(configFilePath)
if err != nil {
t.Fatalf("Error reading modified config file: %v", err)
}

var modifiedConfig map[string]interface{}
err = yaml.Unmarshal(modifiedConfigData, &modifiedConfig)
if err != nil {
t.Fatalf("Error unmarshalling modified config: %v", err)
}

actualEpoch, ok := modifiedConfig["package"].(map[string]interface{})["epoch"].(int)
if !ok {
t.Fatalf("Error retrieving actual epoch from modified config")
}

if actualEpoch != tc.expectedEpoch {
t.Errorf("Epoch not bumped correctly. Expected: %d, Got: %d", tc.expectedEpoch, actualEpoch)
}
})
}
}