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.

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

Vendor Github and Supermarket dependencies #959

Closed
arlimus opened this issue Aug 23, 2016 · 7 comments
Closed

Vendor Github and Supermarket dependencies #959

arlimus opened this issue Aug 23, 2016 · 7 comments
Labels
Type: Enhancement Improves an existing feature
Milestone

Comments

@arlimus
Copy link
Contributor

arlimus commented Aug 23, 2016

Part of #888

Always pull the latest possible release. The user may specify the branch, which results in the latest commit in that branch. The user may also specify a fixed tag or commit.

(Version resolution via semver over tags is not a part of this ticket)

User interface:

- depends:
  - url: github://owner/repo
    [branch: master]
    [tag: nil]
    [ref: nil]
@arlimus arlimus added the Type: Enhancement Improves an existing feature label Aug 23, 2016
@arlimus arlimus mentioned this issue Aug 23, 2016
8 tasks
@chris-rock chris-rock added this to the 0.33.0 milestone Aug 26, 2016
@stevendanna stevendanna self-assigned this Sep 1, 2016
@chris-rock chris-rock modified the milestones: 0.34.0, 0.33.0 Sep 5, 2016
@arlimus
Copy link
Contributor Author

arlimus commented Sep 5, 2016

Decisions to be done on the URL pattern for github.

Option 1

github://owner/repo/branch/branch-name
github://owner/repo/tag/tag-name
github://owner/repo/ref/ref-name

Pro: All in one line, easy to copy
Con: New syntax (afaics); wordy and close to just specifying elements separately

Option 2

NPM/bower git resolver pattern: https://docs.npmjs.com/files/package.json

github://owner/repo#refname

Where refname can be a branch name, tag name, or short/full SHA ref. For git refname see: https://git-scm.com/docs/git-show-ref

Pro: All in one line, easy to copy, short, seen in the wild
Con: Refname can be ambiguous!

@arlimus
Copy link
Contributor Author

arlimus commented Sep 5, 2016

The additional elements that are contained and specified are:

  • branch defaults to master
  • tag
  • ref

In a way, we also have:

Option 3

github://owner/repo

i.e. making it mandatory to specify tag/branch/ref elements manually.

Pro: Explicit, minimal
Con: Needs other attributes, not in one line

Although the last is shaky:

- depends:
  - { url: github://owner/repo, tag: 1.2.3 }

@arlimus
Copy link
Contributor Author

arlimus commented Sep 6, 2016

It's really hard to decide between these three!

However: Option 3 is the only one which doesn't introduce a new syntax that people will depend on.

If we can clearly determine a winner between Options 1 and 2 we still have the chance to offer it as a new feature. For now, these two options are not supported and only Option 3 is offered.

CC: @chris-rock @stevendanna would love your input.

@stevendanna
Copy link
Contributor

stevendanna commented Sep 6, 2016

Here are some thoughts:

Personally, I'm in favor of a path where no heuristics are used to determine the source-type. That is, I'd like to eventually remove the regex-based, "is this a github url?" heuristic we currently have. This would require some deprecation period most likely.

Using the url for everything means that if we ever want to support git-but-not-github sources, then we'll probably need to go with npm-like url schema's that allow them to differentiate between http and git-over-http (they use git+ssh://, git+http://, etc). If we go this route, it is still pretty explicit in terms of what the user intended.

Personally, I like this format:

- depends:
  - url: github://owner/repo
    [branch: master]
    [tag: nil]
    [ref: nil]

or this:

- depends:
  - github: owner/repo
    [branch: master]
    [tag: nil]
    [ref: nil]

Forcing the user to specify branch vs tag vs ref ensures that there is never a question about what they are referring to. But, I definitely see the value in having just a single "url" that embeds all the information as that is easy to copy and paste.

Overall though, I think someone should just play the role of the decider. This is one of those discussions that can go on forever.

@chris-rock
Copy link
Contributor

Lets see what is already there. The following urls are standard git urls as offered by github:

NPM defines:

Note: be aware that npm is not using a real URL for github, gitlab and bitbucket.

I'd like to mention, that we need to distinguish between public github and any other git repo (also Github Enterprise). It's very likely that we would want to offer urls like git://my-github-enterprise-url/user-name/package-name.git, while github:// is a way to get content from github.

I prefer the pure url syntax. We can reuse existing parser and do not need to reinvent the wheel. I still like the current github fetcher, where users just copy paste http urls from github and get it running. I get the point that this requires some heuristics, but its improving the customer convenience.

I really like the npm-like syntax (option 2), where we use a real url style.

Sidenote: Bundler is using option 3 http://bundler.io/git.html

@chris-rock
Copy link
Contributor

chris-rock commented Sep 6, 2016

Another solution we could take:

we use github urls from the browser for github (as the current github fetcher is doing):

- https://github.com/dev-sec/cis-docker-benchmark (master)
- https://github.com/dev-sec/cis-docker-benchmark/tree/chris-rock/attributes (branch)
- https://github.com/dev-sec/cis-docker-benchmark/commit/53f9d4f46da0afa161b0a1fb29aa7b459a6c2174 (ref)
- https://github.com/dev-sec/cis-docker-benchmark/tree/1.1.0 (tag)

and use standard git urls for everything else:

- git@github.com:dev-sec/cis-docker-benchmark.git#ref

The advantage is, that we use the existing github urls, they allow easy copy & paste. There would be no need for a new syntax then

@arlimus
Copy link
Contributor Author

arlimus commented Sep 6, 2016

Git fetcher

In inspec.yml

depends:
- git: https://my-gitlab.com/owner/hello.git
- git: git@my-gitlab.com:owner/hello.git

With these options: branch, tag, ref in inspec.yml:

depends:
- git: https://my-gitlab.com/owner/hello.git
  branch: master
  tag: 1.2.3
  ref: abcef123

The git fetcher uses real-life git under the hood and requires that it is installed! There is no special handling for neither Github, Supermarket, Bitbucket, nor Gitlab!! (i.e. it's just git)

Github fetcher

In inspec.yml we have shorthand fetchers for these targets:

depends:
- github: user/repo

Github is translated into GIT-fetcher urls!.

These options are supported for the github fetcher(only!!): branch, tag, ref in inspec.yml

depends:
- github: user/repo
  branch: master
  tag: 1.2.3
  ref: abcef123

Supermarket fetcher

In inspec.yml:

depends:
- supermarket: user/repo

Supermarket is translated into the URL fetcher (as it currently does not support version enumeration and uses fixed URLs!!).

Private supermarket is supported:

depends:
- supermarket: owner/profile
  supermarket-url: http://my-host.com

URL fetchers

The default URL fetcher has special handling for GitHub URLs. Reasoning: Users want to specify these URLs but they point to a site instead of archives. These URLs are supported in inspec.yml:

depends:
- url: https://github.com/dev-sec/cis-docker-benchmark
- url: https://github.com/dev-sec/cis-docker-benchmark/tree/chris-rock/attributes
- url: https://github.com/dev-sec/cis-docker-benchmark/commit/53f9d4f46da0afa161b0a1fb29aa7b459a6c2174
- url: https://github.com/dev-sec/cis-docker-benchmark/tree/1.1.0

@stevendanna @chris-rock Decision for the UX of this feature for the 1.0 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Enhancement Improves an existing feature
Projects
None yet
Development

No branches or pull requests

3 participants