One refactoring option I can think of would be to make the config for copying of project/application files explicit and use a sane default.

e.g.

variants:
  build:
    copies: { hostfiles: true } # default value so not strictly needed here
  production:
    copies: { from: prep, hostfiles: false } # perhaps the default for `hostfiles:` could be based on `copies.from == nil` so it's not strictly necessary here either
  production2:
    copies: { hostfiles: false } # it would be necessary here since we're defining artifacts explicitly below
    artifacts:
    - from: prep
      source: /srv/app/binfile
      destination: ./

In T211625#4811818, @dduvall wrote:

One refactoring option I can think of would be to make the config for copying of project/application files explicit and use a sane default.

e.g.

variants:
  build:
    copies: { hostfiles: true } # default value so not strictly needed here
  production:
    copies: { from: prep, hostfiles: false } # perhaps the default for `hostfiles:` could be based on `copies.from == nil` so it's not strictly necessary here either
  production2:
    copies: { hostfiles: false } # it would be necessary here since we're defining artifacts explicitly below
    artifacts:
    - from: prep
      source: /srv/app/binfile
      destination: ./

I like the idea of making copying of files explicit. In terms of syntax I think it makes sense to have a list of artifact structs that would now exist under copies. That is, I think the distinction between copies and artifacts is hard to grok. We could allow explicit src and dsts and when those are absent go with a sane default (i.e., for the localhost keyword COPY . .; for copying from variants copy lives.in -> lives.in + LocalLibPrefix -> LocalLibPrefix).

The "hostfiles" keyword is not obvious to me (I think of /etc/hosts for whatever reason). There isn't a great alternative. We could use . or localhost or hostfiles. Maybe hostfolder? I don't know.

I use localhost for the example below.

Example syntax:

build:
  builder: [do, a, thing, to, create, /bin/foo]
production:
  copies:
    - build:
       - src: /bin/foo
         dst: /bin/foo
---
build:
  builder: [do, a, thing, to, make, node_modules]
production:
  copies:
    - localhost
    - build
---
build:
  builder: [do, a, thing, to, make, node_modules]
  builder: [do, a, different, thing, to, create, /bin/foo]
production:
  copies:
    - localhost
    - build
    - build
      - src: /bin/foo
        dst: /bin/foo
---
build:
  copies:
    - localhost
  builder: [create, /bin/foo]
  builder: [populate, node_modules]
production:
  copies: [build]

@thcipriani +1 to the proposal in general. I think it adds clarity to artifact definition and to the purpose of copies. Just some notes on config structs and implementation of sane defaults.

Syntax

My initial reaction to allowing for both forms:

copies:
 - build

and:

copies:
 - build:
    - source: /bin/foo
    - destination: /bin/foo

is that it's nonobvious, and that reaction is probably from working with similar patterns in JJB. In JJB, I've never like the fact that it introduces essentially two more layers of nested structure (from list[string] to list[map[string: list[map[string: string]]]] to allow for the sake of a certain readable form when only one additional layer is needed to actually store the additional information (from list[string] to list[map[string: string]]).

Maybe that's a little pedantic, but I think there's a compromise that's still readable, which would be these two forms:

copies:
 - build

and:

copies:
 - from: build
   source: /bin/foo
   destination: /bin/foo

It still allows for a short form, which I think is important since it's close to what we currently support for copies (most compactly copies: [build]), and the long form is more efficient in its data representation and parsing.

Supporting either sets of short/long forms will require a little bit of extra logic in our parser, but I think we can and should do it for the sake of supporting the short form.

Defaults

For defaults, in order to be backwards compatible, we'd have to support the following interpretations/expansions. I'll use local in my examples to signify the host directory context, making it my official vote for a keyword. :)

Example

copies: [local]

should be expanded to

copies:
 - from: local
   source: .
   destination: .

Example

copies: [foo] # anything other than local

should be expanded to

copies:
 - from: foo
   source: /srv/app # assuming lives.in is left as the /srv/app default
   destination: /srv/app
 - from: foo
   source: /opt/lib
   destination: /opt/lib

Change 485340 had a related patch set uploaded (by Dduvall; owner: Dduvall):
[blubber@master] Unify copies and artifacts configuration

https://gerrit.wikimedia.org/r/485340

Change 485340 merged by jenkins-bot:
[blubber@master] Unify copies and artifacts configuration

https://gerrit.wikimedia.org/r/485340