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.

Sign up for GitHub

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

Jump to bottom

feat(cmd): add index create subcommand to create an external carv2 index #350

Merged
merged 2 commits into from
Feb 1, 2023
Merged

feat(cmd): add index create subcommand to create an external carv2 index #350

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7df51ce
feat(cmd): add index create subcommand to create an external carv2 index
aschmahmann Jan 16, 2023
4038bee
test(cmd): add index creation test
aschmahmann Jan 30, 2023
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
5 changes: 5 additions & 0 deletions cmd/car/car.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,11 @@ func main1() int {
Usage: "Write output as a v1 or v2 format car",
},
},
Subcommands: []*cli.Command{{
Name: "create",
Usage: "Write out a detached index",
Action: CreateIndex,
}},
},
{
Name: "inspect",
Expand Down
42 changes: 42 additions & 0 deletions cmd/car/index.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,3 +166,45 @@ func IndexCar(c *cli.Context) error {
_, err = index.WriteTo(idx, outStream)
return err
}

// CreateIndex is a command to write out an index of the CAR file
func CreateIndex(c *cli.Context) error {
r, err := carv2.OpenReader(c.Args().Get(0))
if err != nil {
return err
}
defer r.Close()

outStream := os.Stdout
if c.Args().Len() >= 2 {
outStream, err = os.Create(c.Args().Get(1))
if err != nil {
return err
}
}
defer outStream.Close()

var mc multicodec.Code
if err := mc.Set(c.String("codec")); err != nil {
return err
}
idx, err := index.New(mc)
if err != nil {
return err
}

dr, err := r.DataReader()
if err != nil {
return err
}

if err := carv2.LoadIndex(idx, dr); err != nil {
return err
}

if _, err := index.WriteTo(idx, outStream); err != nil {
return err
}

return nil
}
3 changes: 3 additions & 0 deletions cmd/car/testdata/script/index-create.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
car index create ${INPUTS}/sample-v1.car sample-v1.car.idx
car detach-index ${INPUTS}/sample-wrapped-v2.car sample-wrapped-v2.car.idx
cmp sample-v1.car.idx sample-wrapped-v2.car.idx
Comment on lines +1 to +3
Copy link
Contributor Author

Choose a reason for hiding this comment

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

lmk if this is too simple and I can either add an index to the inputs to compare against or add something like:

car detach-index list sample-v1.car.idx
cmp stdout index-list.txt

-- index-list.txt --
...

Copy link
Member

Choose a reason for hiding this comment

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

no, I think this is fine; we're not exactly doing sophisticated and high-coverage tests here in cmd

this does make me think though that maybe detach should be moved to a subcmd of index too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this does make me think though that maybe detach should be moved to a subcmd of index too

I agree, but figured I'd let the maintainers make that call and if they'd be ok with a breaking move or want both supported.

Copy link
Member

Choose a reason for hiding this comment

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

we don't exactly have an SLA on this, cmd is currently just a grab-bag of utils we collectively find useful

Copy link
Contributor Author

Choose a reason for hiding this comment

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

SGTM. Happy to submit a PR to move it once this one is closed out