sifive / wit

Workspace Integration Tool
Apache License 2.0
23 stars 13 forks source link

Question about behavior of transitive dependencies #3

Open jackkoenig opened 5 years ago

jackkoenig commented 5 years ago

Given a project structure like: fizz depends on bar bar depends on foo

My understanding is that fizz would end up with both bar and foo as subdirectories but this doesn't appear to be the case. The following bash script shows what I mean:

#!/usr/bin/env bash

DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"

cd $DIR
wit create foo
cd foo
git init
git add wit_manifest.json
git commit -m "Initial commit"

cd $DIR
wit create bar
cd bar
wit add $DIR/foo
git init
git add wit_manifest.json
git commit -m "Initial commit"

cd $DIR
wit create fizz
cd fizz
wit add $DIR/bar
git init
git add wit_manifest.json
git commit -m "Initial commit"
wit update

If you run the above, you get:

INFO:wit:Checking [/scratch/koenig/wit/experiment/fizz/wit_manifest.json]
INFO:wit:Updating workspace
INFO:wit:No dependency file found in repo [1fc06b9979c4126fe52a40e59053cc8e5824044a:/scratch/koenig/wit/experiment/fizz/bar]
Repo name [bar] commit [1fc06b9979c4126fe52a40e59053cc8e5824044a]

and

$ ls fizz
bar/  wit_manifest.json

I would expect there to be a foo/ directory within fizz, am I misunderstanding something?

samh-sifive commented 5 years ago

With the wisdom of hindsight the current model is a bit confusing.

wit add adds to the wit_manifest file. But what it actually cares about in sub-repos is a wit_dependencies file. That file is json, and looks like

$ cat soc/a/wit_dependencies.json
[
    {
        "name": "b",
        "path": "/work/samh/ws/b.git",
        "hash": "d732747c"
    }
]

This is confusing and now that I think about it, totally wrong. The workspace should have its own wit_dependencies file, and the wit_manifest file should be purely generated.

So the fix will be:

  1. wit init will create an empty wit_dependencies.json file.
  2. wit add will add to the wit_dependencies.json file
  3. I'll split wit update into two commands, wit resolve and wit checkout. wit update will still exist, and just run both of these.
  4. wit resolve will resolve all the dependencies and create wit_manifest.json
  5. wit checkout will read the wit_manifest.json and do all the checkouts.
jackkoenig commented 5 years ago

Clarifying questions:

In general use, you should only commit the wit_manifest.json file because that describes the dependencies in a more flexible way while wit_dependencies.json is more of the concrete "this is what is checked out" right?

And when you wit add a repo with just a wit_manifest.json, what happens? Does it resolve dependencies or do you have to run one of the new commands?

samh-sifive commented 5 years ago

Actually the opposite. The intention is for wit_dependencies.json to list what you require (e.g. fizz requires foo 1.0 and bar 1.1.

wit_manifest.json gets populated with what is actually resolved. For example if foo requires bar 1.2, then even though soc has bar 1.1 in its dependency, wit_manifest.json will show bar 1.2, and that will be checked out.

So the intention is that wit_manifest.json contains what is actually present in the workspace. This is a little different than what we discussed over the phone today. wit also needs to have the ability to read in the manifest file and simply check out exactly what it states without re-doing the resolution (even though the wit_manifest.json should be able to be deterministically re-created based on the soc wit_dependencies.json.

richardxia commented 5 years ago

Excuse my drive-by commenting, especially because I don't have much context on exactly what you guys are doing, but based on @samh-sifive's most recent comment, wit_manifest.json sounds analogous to what several languages call a "lock" file. If this is indeed what it's meant to do, I'd suggest putting the word "lock" in its name, since that would make it immediately recognizable to anyone that's used a language/package manager that follows that convention.

Examples of languages or package managers that call it a lock file:

samh-sifive commented 5 years ago

I appreciate the comment @richardxia ! I've always considered lock-files in the semaphore-sense to prevent resource contention, but a little googling confirms your stance as it pertains to package systems. This:

https://medium.com/@sdboyer/so-you-want-to-write-a-package-manager-4ae9c17d9527

suggests that what I've been calling wit_dependencies.json should actually be wit_manifest.json, and what I've been calling wit_manifest.json should actually be wit_lock.json. Thanks for making this suggestion, this is obviously the right time to fix my terminology :). I'll also replace _ with -, as that seems to be more the norm.

So the corrected behavior is then:

samh-sifive commented 5 years ago
% module load sifive/wit/0.2
Loading module for wit 0.2.

% ./test fizz
Creating workspace [foo]
Initialized empty Git repository in /work/samh/wit.test/foo/.git/
[master (root-commit) 86ce4e7] Initial commit
 1 file changed, 1 insertion(+)
 create mode 100644 wit-manifest.json
Creating workspace [bar]
INFO:wit:Checking [/work/samh/wit.test/bar/wit-manifest.json]
INFO:wit:Adding repo to workspace
Adding [/home/samh/w/wit.test/foo] to manifest as [foo]
Initialized empty Git repository in /work/samh/wit.test/bar/.git/
[master (root-commit) e21bca8] Initial commit
 1 file changed, 7 insertions(+)
 create mode 100644 wit-manifest.json
Creating workspace [fizz]
INFO:wit:Checking [/work/samh/wit.test/fizz/wit-manifest.json]
INFO:wit:Adding repo to workspace
Adding [/home/samh/w/wit.test/bar] to manifest as [bar]
Initialized empty Git repository in /work/samh/wit.test/fizz/.git/
[master (root-commit) 8be3443] Initial commit
 1 file changed, 7 insertions(+)
 create mode 100644 wit-manifest.json
INFO:wit:Checking [/work/samh/wit.test/fizz/wit-manifest.json]
INFO:wit:Updating workspace
Repo name [bar] commit [e21bca8e92752db30567aa0d80bfc8e7e8ba7641]
Repo name [foo] commit [86ce4e7e7afe06bde0f7a846da52d5e2c177e233]

% cat fizz/wit-lock.json
{
    "bar": {
        "commit": "e21bca8e92752db30567aa0d80bfc8e7e8ba7641",
        "name": "bar",
        "source": "/home/samh/w/wit.test/bar"
    },
    "foo": {
        "commit": "86ce4e7e7afe06bde0f7a846da52d5e2c177e233",
        "name": "foo",
        "source": "/home/samh/w/wit.test/foo"
    }
}
aaronjanse commented 5 years ago

Is this resolved? @jackkoenig