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

Define build time dependencies #181

Merged
merged 10 commits into from
Jun 14, 2021
+39 −2
Merged

Define build time dependencies #181

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
c93c184
Define build time dependencies
Jun 2, 2021
97435a3
Update changelog
Jun 2, 2021
a8915b1
Address PR comments
mtojek Jun 7, 2021
10552d7
Merge branch 'master' into 179-build-dependencies
Jun 9, 2021
ef12409
Extract to deps.yaml
Jun 9, 2021
c5c33aa
Rename deps.yml to build.yml
Jun 10, 2021
7ccc2aa
Move build.yml under _dev/build
Jun 10, 2021
57e9fba
Merge branch 'master' into 179-build-dependencies
Jun 10, 2021
b532033
Replace array with obj
Jun 10, 2021
ec81ed5
Fix
Jun 10, 2021
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
2 changes: 1 addition & 1 deletion code/go/internal/spec/statik.go
View file
Open in desktop

Large diffs are not rendered by default.

3 changes: 3 additions & 0 deletions test/packages/good/manifest.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,6 @@ icons:
title: system
size: 1000x1000
type: image/svg+xml
buildDependencies:
- name: ecs
reference: git@7a11f3e1f7d8f05e8f06623fd98254b8cb34c441
jsoriano marked this conversation as resolved.
Show resolved Hide resolved
5 changes: 4 additions & 1 deletion versions/1/changelog.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,4 +126,7 @@
link: https://github.com/elastic/package-spec/pull/175
- description: Define common test config for pipeline tests
type: enhancement
link: https://github.com/elastic/package-spec/pull/176
link: https://github.com/elastic/package-spec/pull/176
- description: Define build-time dependencies
type: enhancement
link: https://github.com/elastic/package-spec/pull/181
19 changes: 19 additions & 0 deletions versions/1/manifest.spec.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,25 @@ spec:
pattern: '^(([a-zA-Z0-9-]+)|([a-zA-Z0-9-]+\/[a-zA-Z0-9-]+))$'
required:
- github
buildDependencies:
mtojek marked this conversation as resolved.
Show resolved Hide resolved
type: array
description: Build-time dependencies
items:
type: object
additionalProperties: false
properties:
name:
type: string
description: Name of the dependency
enum:
- ecs
reference:
type: string
description: Source reference
pattern: 'git@[a-z0-9]{40}'
Copy link
Member

Choose a reason for hiding this comment

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

Allow the use of tags too? (Can be extended in the future in any case)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason why I chose a commit hash is because it's always the same for all develops. If we put a tag here (e.g. [email protected]), the elastic-package will build a slightly different package based on the commit marked with the tag. I was thinking about a switch for elastic-package build, for example: --update, which can update references to latest dependency versions.

Actually I see a problem with this design: how can I pick the latest commit for ECS 1.9? Let me suggest something.

Copy link
Contributor Author

@mtojek mtojek Jun 7, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Ok, I adjusted the manifest to include additional field "version". The update procedure would be as follows:

  1. Check ECS dependency considering version (e.g. 1.9).
  2. Fetch latest commit from the branch 1.9.
  3. Update reference with the commit hash.

Note:

I'm not fully satisfied with this approach (need to store version and source reference), but I don't see any other reasonable approach. We can always store an additional metafile somewhere around, but with this decision we could also introduce a special deps.yml instead of keeping everything in package manifest.

Copy link
Member

@jsoriano jsoriano Jun 7, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

@mtojek having a version and a reference can be confusing, even contradictory if someone sets a reference that doesn't correspond to a version. I think that we can go on by now with your original approach with the reference only, and we can improve it later to support other use cases.

how can I pick the latest commit for ECS 1.9?

Fetch latest commit from the branch 1.9.

I wouldn't try to support branches, at least by now, configured branches may lead to unreproducible builds, same code built at different moments can result in packages with different fields.

Copy link
Contributor

Choose a reason for hiding this comment

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

++

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

required:
- name
- reference
required:
- format_version
- name
Expand Down