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

add in a default file hash and cleaned up init function a bit #266

Merged
merged 4 commits into from
Nov 16, 2014
Merged

add in a default file hash and cleaned up init function a bit #266

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ab805f3
add in a default file hash and cleaned up init functiona bit
whyrusleeping Nov 4, 2014
e857a5b
move default hash into ipfs2, committed on freemont troll
whyrusleeping Nov 16, 2014
7596bcc
cleaned up ipfs init
jbenet Nov 16, 2014
ab09c36
defaulthash: let's not ignore errors
jbenet Nov 16, 2014
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
137 changes: 95 additions & 42 deletions cmd/ipfs/init.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,22 @@
package main

import (
"bytes"
"encoding/base64"
"errors"
"fmt"
"os"
"path/filepath"

"github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/gonuts/flag"
"github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/commander"
config "github.com/jbenet/go-ipfs/config"
core "github.com/jbenet/go-ipfs/core"
ci "github.com/jbenet/go-ipfs/crypto"
imp "github.com/jbenet/go-ipfs/importer"
chunk "github.com/jbenet/go-ipfs/importer/chunk"
peer "github.com/jbenet/go-ipfs/peer"
updates "github.com/jbenet/go-ipfs/updates"
u "github.com/jbenet/go-ipfs/util"
)

Expand All @@ -33,6 +39,66 @@ func init() {
cmdIpfsInit.Flag.String("d", "", "Change default datastore location")
}

var defaultPeers = []*config.BootstrapPeer{
&config.BootstrapPeer{
// mars.i.ipfs.io
PeerID: "QmaCpDMGvV2BGHeYERUEnRQAwe3N8SzbUtfsmvsqQLuvuJ",
Address: "/ip4/104.131.131.82/tcp/4001",
},
}

func datastoreConfig(dspath string) (config.Datastore, error) {
ds := config.Datastore{}
if len(dspath) == 0 {
var err error
dspath, err = config.DataStorePath("")
if err != nil {
return ds, err
}
}
ds.Path = dspath
ds.Type = "leveldb"

// Construct the data store if missing
if err := os.MkdirAll(dspath, os.ModePerm); err != nil {
return ds, err
}

// Check the directory is writeable
if f, err := os.Create(filepath.Join(dspath, "._check_writeable")); err == nil {
os.Remove(f.Name())
} else {
return ds, errors.New("Datastore '" + dspath + "' is not writeable")
}

return ds, nil
}

func identityConfig(nbits int) (config.Identity, error) {
ident := config.Identity{}
fmt.Println("generating key pair...")
sk, pk, err := ci.GenerateKeyPair(ci.RSA, nbits)
if err != nil {
return ident, err
}

// currently storing key unencrypted. in the future we need to encrypt it.
// TODO(security)
skbytes, err := sk.Bytes()
if err != nil {
return ident, err
}
ident.PrivKey = base64.StdEncoding.EncodeToString(skbytes)

id, err := peer.IDFromPubKey(pk)
if err != nil {
return ident, err
}
ident.PeerID = id.Pretty()

return ident, nil
}

func initCmd(c *commander.Command, inp []string) error {
configpath, err := getConfigDir(c.Parent)
if err != nil {
Expand Down Expand Up @@ -62,30 +128,12 @@ func initCmd(c *commander.Command, inp []string) error {
}
cfg := new(config.Config)

cfg.Datastore = config.Datastore{}
if len(dspath) == 0 {
dspath, err = config.DataStorePath("")
if err != nil {
return err
}
}
cfg.Datastore.Path = dspath
cfg.Datastore.Type = "leveldb"

// Construct the data store if missing
if err := os.MkdirAll(dspath, os.ModePerm); err != nil {
// setup the datastore
cfg.Datastore, err = datastoreConfig(dspath)
if err != nil {
return err
}

// Check the directory is writeable
if f, err := os.Create(filepath.Join(dspath, "._check_writeable")); err == nil {
os.Remove(f.Name())
} else {
return errors.New("Datastore '" + dspath + "' is not writeable")
}

cfg.Identity = config.Identity{}

// setup the node addresses.
cfg.Addresses = config.Addresses{
Swarm: "/ip4/0.0.0.0/tcp/4001",
Expand All @@ -106,42 +154,47 @@ func initCmd(c *commander.Command, inp []string) error {
return errors.New("Bitsize less than 1024 is considered unsafe.")
}

u.POut("generating key pair\n")
sk, pk, err := ci.GenerateKeyPair(ci.RSA, nbits)
cfg.Identity, err = identityConfig(nbits)
if err != nil {
return err
}

// currently storing key unencrypted. in the future we need to encrypt it.
// TODO(security)
skbytes, err := sk.Bytes()
if err != nil {
return err
// Use these hardcoded bootstrap peers for now.
cfg.Bootstrap = defaultPeers

// tracking ipfs version used to generate the init folder
// and adding update checker default setting.
cfg.Version = config.Version{
Check: "error",
Current: updates.Version,
}
cfg.Identity.PrivKey = base64.StdEncoding.EncodeToString(skbytes)

id, err := peer.IDFromPubKey(pk)
err = config.WriteConfigFile(filename, cfg)
if err != nil {
return err
}
cfg.Identity.PeerID = id.Pretty()
u.POut("peer identity: %s\n", id.Pretty())

// Use these hardcoded bootstrap peers for now.
cfg.Bootstrap = []*config.BootstrapPeer{
&config.BootstrapPeer{
// mars.i.ipfs.io
PeerID: "QmaCpDMGvV2BGHeYERUEnRQAwe3N8SzbUtfsmvsqQLuvuJ",
Address: "/ip4/104.131.131.82/tcp/4001",
},
nd, err := core.NewIpfsNode(cfg, false)
if err != nil {
return err
}
defer nd.Close()
Copy link
Contributor

Choose a reason for hiding this comment

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

does the node need to be closed everywhere it is instantiated?

Copy link
Member Author

Choose a reason for hiding this comment

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

I would feel better if it was, technically it doesnt need to be, because in most of our uses, no longer needing the node means closing down the program and letting the operating system clean up, but Im always slightly paranoid

Copy link
Contributor

Choose a reason for hiding this comment

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

i see. we may know better, but we can't rely on our individual knowledge. developers who import our library and instantiate nodes programmatically may have other plans. the system needs to speak for itself.

perhaps we should communicate this through the function signature:

node, cancel, err := core.Node(...)

where the implementation does the following:

ctx, cancel := ...

... create ...

return node, cancel, nil

Copy link
Member

Choose a reason for hiding this comment

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

would feel better if it was

agreed.

node, cancel, err := core.Node(...)

👎 leaking underlying libs and abstractions is not clean. This is what the Closer interface is for.

node, err :=  ipfs.NewNode(...)
defer node.Close()
...

Internally, that Close method can do whatever, but our interfaces do not leak out the entrails of our implementations.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's incorrect. cancel is a func(). That is not leaking an abstraction. It's explanatory, and forces the caller to make a conscious decision to ignore shutdown. Much safer practice than depending on the programmer to call defer Close() at all call sites.

Design the system to minimize programmer error. This only increases in importance as the system grows.

Good interfaces are:
Easy to use correctly. People using a well-designed interface almost always use the interface correctly, because that's the path of least resistance. In an API, they almost always pass the correct parameters with the correct values, because that's what's most natural. With interfaces that are easy to use correctly, things just work.
Hard to use incorrectly. Good interfaces anticipate mistakes people might make and make them difficult — ideally impossible — to commit.

people might forget to close => make it impossible to forget

We have empirical evidence that the existing interface is easy to use incorrectly.

Copy link
Member Author

Choose a reason for hiding this comment

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

ehh.... im still not super sold on the returning a cancel func idea... It doesnt feel like idiomatic Go to me.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fair. I stand by merits of the approach, but accept and acknowledge its downsides. I'm not sold either. I'm okay with continuing to manage the node the same way we do files and locks.

Copy link
Member Author

Choose a reason for hiding this comment

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

In a way, its the same as making people remember to close a file, or a socket

Copy link
Member

Choose a reason for hiding this comment

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

forces the caller to make a conscious decision to ignore shutdown. Much safer practice than depending on the programmer to call defer Close() at all call sites.

I agree that is a nice property. An approach like:

unlock := lock.Lock()
doSomethingCritical()
unlock()

Might avoid making mistakes a bit more. But note it doesn't actually force the user to deal with the return value.

lock.Lock()
doSomethingCritical
// oops

doesn't even get a warning. So it's not an 100% solution.

The added complexity and leaking of abstractions is that if cancel its not going to be called in the same function than you have to continue leaking it out, or add it to a new object:

// you have to carry around an additional cancel _everywhere_ you pass n.
// now prone to programming and design error.
// the function is now state you're passing around. hiding it in a callstack does not change that.
func constructNode() ipfs.IPFS, context.CancelFunc {
  cfg := ....
  ...
  ...
  n, cancel := ipfs.NewNode(...)
  ...
  ...
  ...
  return n, cancel 
}

// or what will invariably happen:

type nodeCtx struct {
  node ipfs.IPFS
  cancel context.CancelFunc
}


func constructNode() nodeCtx {
  cfg := ....
  ...
  ...
  n, cancel := ipfs.NewNode(...)
  ...
  ...
  ...
  return nodeCtx{node: n, cancel: cancel}
}

I dont think passing these cancel functions around is a good idea. I'd be curious to see what go-nuts have to say about it.

This is not critical. Let's keep it in mind and move on.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed.


// tracking ipfs version used to generate the init folder and adding update checker default setting.
cfg.Version = config.VersionDefaultValue()
// Set up default file
msg := `Hello and Welcome to IPFS!
Copy link
Contributor

Choose a reason for hiding this comment

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

This message hasn't been applied to the new PR @whyrusleeping

Copy link
Member Author

Choose a reason for hiding this comment

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

@maybebtc Which one is the new PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

#263 @whyrusleeping

If you're seeing this, that means that you have successfully
installed ipfs and are now interfacing with the wonderful
world of DAGs and hashes!
`
reader := bytes.NewBufferString(msg)

err = config.WriteConfigFile(filename, cfg)
defnd, err := imp.BuildDagFromReader(reader, nd.DAG, nd.Pinning.GetManual(), chunk.DefaultSplitter)
if err != nil {
return err
}

k, _ := defnd.Key()
fmt.Printf("Default file key: %s\n", k)

return nil
}
27 changes: 27 additions & 0 deletions cmd/ipfs2/init.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package main

import (
"bytes"
"encoding/base64"
"errors"
"fmt"
Expand All @@ -9,7 +10,10 @@ import (

cmds "github.com/jbenet/go-ipfs/commands"
config "github.com/jbenet/go-ipfs/config"
core "github.com/jbenet/go-ipfs/core"
ci "github.com/jbenet/go-ipfs/crypto"
imp "github.com/jbenet/go-ipfs/importer"
chunk "github.com/jbenet/go-ipfs/importer/chunk"
peer "github.com/jbenet/go-ipfs/peer"
u "github.com/jbenet/go-ipfs/util"
)
Expand Down Expand Up @@ -114,6 +118,29 @@ func doInit(configRoot string, dspathOverride string, force bool, nBitsForKeypai
if err != nil {
return err
}

nd, err := core.NewIpfsNode(&conf, false)
if err != nil {
return err
}
defer nd.Close()

// Set up default file
msg := `Hello and Welcome to IPFS!
If you're seeing this, that means that you have successfully
installed ipfs and are now interfacing with the wonderful
world of DAGs and hashes!
`
reader := bytes.NewBufferString(msg)

defnd, err := imp.BuildDagFromReader(reader, nd.DAG, nd.Pinning.GetManual(), chunk.DefaultSplitter)
if err != nil {
return err
}

k, _ := defnd.Key()
fmt.Printf("Default file key: %s\n", k)
Copy link
Member

Choose a reason for hiding this comment

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

though init is local, we've moved to not have any Printf anywhere except main. Return the string and have it printed as usual cmd output

Copy link
Member

Choose a reason for hiding this comment

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

nevermind, init already prints this way... >.<


return nil
}

Expand Down
5 changes: 5 additions & 0 deletions pin/pin.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ type Pinner interface {
Pin(*mdag.Node, bool) error
Unpin(util.Key, bool) error
Flush() error
GetManual() ManualPinner
}

// ManualPinner is for manually editing the pin structure
Expand Down Expand Up @@ -263,3 +264,7 @@ func (p *pinner) PinWithMode(k util.Key, mode PinMode) {
p.indirPin.Increment(k)
}
}

func (p *pinner) GetManual() ManualPinner {
return p
}