cmd: migrate 'surgery copy-page' command to cobra style comamnd

[ahrtr]

Linked to #472

[ahrtr]

@mitake @ptabor @serathius @spzala

[ptabor]

Shell we warn if the freelist is persisted and recommend dropping it ?

This might cause unreachable pages being reachable again.

[ahrtr]

Good point! Added,

	fmt.Fprintf(os.Stdout, "WARNING: the free list might have changed.\n")
	fmt.Fprintf(os.Stdout, "Please consider executing `./bbolt surgery abandon-freelist ...`\n")

[ahrtr]

thx @ptabor

[ptabor]

Thank you for quick fix.
Ideally it should be a helper function that checks whether freelist is enabled before printing this warning.

[ahrtr]

Yes, let me do a minor refactoring in a followup PR,

bbolt/cmd/bbolt/command_surgery_cobra.go

Lines 218 to 222 in 88c6eb7

	meta, err := readMetaPage(srcDBPath)
	if err != nil {
	return err
	}
	if meta.Freelist() != common.PgidNoFreelist {

[ptabor]

Thank you for quick fix.
Ideally it should be a helper function that checks whether freelist is enabled before printing this warning.

[serathius]

Let's use Kubernetes flag pattern, it's much more extensible and compositable.

Suggested change

	func newSurgeryCopyPageCommand() *cobra.Command {
	copyPageCmd := &cobra.Command{
	Use: "copy-page <bbolt-file> [options]",
	Short: "Copy page from the source page Id to the destination page Id",
	Args: func(cmd *cobra.Command, args []string) error {
	if len(args) == 0 {
	return errors.New("db file path not provided")
	}
	if len(args) > 1 {
	return errors.New("too many arguments")
	}
	return nil
	},
	RunE: surgeryCopyPageFunc,
	}
	copyPageCmd.Flags().StringVar(&surgeryTargetDBFilePath, "output", "", "path to the target db file")
	copyPageCmd.Flags().Uint64VarP(&surgerySourcePageId, "from-page", "", 0, "source page Id")
	copyPageCmd.Flags().Uint64VarP(&surgeryDestinationPageId, "to-page", "", 0, "destination page Id")
	return copyPageCmd
	}
	func surgeryCopyPageFunc(cmd *cobra.Command, args []string) error {
	srcDBPath := args[0]
	if surgerySourcePageId == surgeryDestinationPageId {
	return fmt.Errorf("'--from-page' and '--to-page' have the same value: %d", surgerySourcePageId)
	}
	if err := common.CopyFile(srcDBPath, surgeryTargetDBFilePath); err != nil {
	return fmt.Errorf("[copy-page] copy file failed: %w", err)
	}
	if err := surgeon.CopyPage(surgeryTargetDBFilePath, common.Pgid(surgerySourcePageId), common.Pgid(surgeryDestinationPageId)); err != nil {
	return fmt.Errorf("copy-page command failed: %w", err)
	}
	type surgeryOptions struct {
	surgeryTargetDBFilePath string
	surgeryPageId uint64
	surgeryStartElementIdx int
	surgeryEndElementIdx int
	surgerySourcePageId uint64
	surgeryDestinationPageId uint64
	}
	func defaultSurgeryOptions() surgeryOptions {
	return surgeryOptions{}
	}
	func (o *surgeryOptions) Validate() error {
	if o.surgerySourcePageId == o.surgeryDestinationPageId {
	return fmt.Errorf("'--from-page' and '--to-page' have the same value: %d", surgerySourcePageId)
	}
	}
	func (o *Options) AddFlags(fs flag.FlagSet) {
	fs.StringVar(&o.surgeryTargetDBFilePath, "output", o.surgeryTargetDBFilePath, "path to the target db file")
	fs.Uint64Var(&o.surgerySourcePageId, "from-page", o.surgerySourcePageId, "source page Id")
	fs.Uint64Var(&o.surgeryDestinationPageId, "to-page", o.surgeryDestinationPageId, "destination page Id")
	}
	func newSurgeryCopyPageCommand() *cobra.Command {
	cfg := defaultSurgeryOptions()
	cmd := &cobra.Command{
	Use: "copy-page <bbolt-file> [options]",
	Short: "Copy page from the source page Id to the destination page Id",
	Args: func(c *cobra.Command, args []string) error {
	if len(args) == 0 {
	return errors.New("db file path not provided")
	}
	if len(args) > 1 {
	return errors.New("too many arguments")
	}
	return nil
	},
	RunE: func(cmd *cobra.Command, args []string) error {
	err := o.Validate()
	if err != nil {
	return err
	}
	return surgeryCopyPageFunc(args[0], o)
	},
	}
	o.AddFlags(cmd.Flags())
	return cmd
	}
	func surgeryCopyPageFunc(srcDBPath string, o *surgeryOptions) error {
	if err := common.CopyFile(srcDBPath, surgeryTargetDBFilePath); err != nil {
	return fmt.Errorf("[copy-page] copy file failed: %w", err)
	}
	if err := surgeon.CopyPage(surgeryTargetDBFilePath, common.Pgid(surgerySourcePageId), common.Pgid(surgeryDestinationPageId)); err != nil {
	return fmt.Errorf("copy-page command failed: %w", err)
	}
	fmt.Fprintf(os.Stdout, "The page %d was successfully copied to page %d\n", surgerySourcePageId, surgeryDestinationPageId)
	return nil
	}

[ahrtr]

The struct surgeryOptions makes sense, but not the (*surgeryOptions) Validate(), because different surgery commands have different validation logic.

Let me apply the struct surgeryOptions together @ptabor 's comment above in a separate PR. Regarding to K8s flag pattern, I do not see any essential difference or obvious benefits. But please feel free to raise a separate issue or draft PR to discuss it. thx

[ahrtr]

After second look, your suggestion indeed is better, it removes all the global variables. Resolved this comment in #483 . thx