geofffranks / spruce

A BOSH template merge tool
MIT License
433 stars 78 forks source link

cherry-pick doesn't seem to be happening at final stage, as advertised #344

Open phlummox opened 3 years ago

phlummox commented 3 years ago

According to the merging rules, cherry-picking should happen as the last-but-one stage of merging. And "--cherry-pick" and "--prune" are described in the command --help as opposites of each other. However, they seem to give very different results in the following case, and whether cherry-picking is done seems to alter the behaviour of earlier stages.

Assume we have the following files:

recipe.yaml:

name: text and suet pudding
ingredients:
- 1 cup chopped suet
- 3 cups flour
- 1 cup milk
- bits_of_text:
    (( grab some_text ))
method:
- combine thoroughly
- serve with parsley garnish

text.yaml:

some_text: 
- (( grab files.lorem ))

files:
  lorem: (( file "lorem.txt" ))

and lorem.txt:

lorem ipsum dolor sit amet

If we run

$ spruce merge --prune files --prune some_text recipe.yml text.yml

then we get the expected output:

ingredients:
- 1 cup chopped suet
- 3 cups flour
- 1 cup milk
- bits_of_text:
  - |+
    lorem ipsum dolor sit amet

method:
- combine thoroughly
- serve with parsley garnish
name: text and suet pudding

But if we instead try and cherry-pick the "name", "method" and "ingredients" keys, we get quite different output, and the "file" operator in text.yml doesn't seem to have been executed at all. Executing

$ spruce merge --cherry-pick name --cherry-pick method --cherry-pick ingredients recipe.yml text.yml

gives the output:

ingredients:
- 1 cup chopped suet
- 3 cups flour
- 1 cup milk
- bits_of_text:
  - (( grab files.stack_build_script ))
method:
- combine thoroughly
- serve with parsley garnish
name: text and suet pudding

So something seems to be wrong - either the documentation isn't correct that "cherry-pick" happens at the penultimate stage, or the merge isn't being done according to the semantics given. (Or I'm encountering some sort of undefined behaviour due to misuse of the operators, but if so, that's not obvious to me.)

I'm using version 1.28.0 of spruce, downloaded from https://github.com/geofffranks/spruce/releases/download/v1.28.0/spruce-linux-amd64.

geofffranks commented 3 years ago

I created a sample of this on play.spruce.cf, with tracing enabled. it seems like spruce isn't recognizing that some_text is an operator.

DEBUG> running (( grab ... )) operation at $.ingredients.3.bits_of_text
 DEBUG>   arg[0]: trying to resolve reference $.some_text
 DEBUG>      [0]: resolved to a value (could be a map, a list or a scalar);
              → appending
geofffranks commented 3 years ago

Looks like https://github.com/geofffranks/spruce/commit/265eb84025080b2e32ccfc5bee387465c3fe6b62 solved this slightly. Older versions didn't resolve any operators. That resolved one level. Need to make it more recursive so any dependencies are pulled in.

geofffranks commented 3 years ago

Feel free to PR for this if you want @phlummox!

phlummox commented 3 years ago

I'm afraid I don't know Go, so I don't think there's anything I'd be able to usefully provide in a PR. Best of luck resolving the issue.