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

Add Lua Rockspec package type #18

Merged
merged 2 commits into from
Mar 12, 2024

Conversation

LaurentGoderre
Copy link

@coveralls
Copy link

coveralls commented Feb 8, 2024

Pull Request Test Coverage Report for Build 7834942742

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 80.087%

Totals Coverage Status
Change from base Build 7759292336: 0.0%
Covered Lines: 185
Relevant Lines: 231

💛 - Coveralls

@willmurphyscode
Copy link

I looks like over at https://github.com/package-url/purl-spec/blob/master/PURL-TYPES.rst#other-candidate-types-to-define, the candidate for LuaRocks packages is lua not rockspec, but it's just a candidate. I didn't find any tracking issue on that repo either, so I think even if we change this to lua there's some risk that a different string gets standardized on later.

@wagoodman what do you think here? Should we change it to lua?

@LaurentGoderre
Copy link
Author

I went wity rockspec because it's to Lua what npm is to JavaScript.

@LaurentGoderre
Copy link
Author

Maybe LuaRock would be better to differentiate it from the language?

@wagoodman
Copy link

wagoodman commented Feb 28, 2024

The only hints from the spec we get are:

type:

The package type is composed only of ASCII letters and numbers, '.', '+' and '-' (period, plus, and dash)
The type cannot start with a number
The type cannot contains spaces
The type must NOT be percent-encoded
The type is case insensitive. The canonical form is lowercase

and here:

The package type is the component of a package URL that is used to capture this information with a short string such as maven, npm, nuget, gem, pypi, etc.
These are known purl package type definitions.
Known purl type definitions are formalized here independent of the core Package URL specification. See also a candidate list further down.
Definitions can also include types reserved for future use.

There are some other issues in the purl spec repo that get close to this, but couldn't find any exact matches.

I did find a PR comment that has an opinion here:

I think of the type as more representative of "packaging format", which cargo seems to represent fine, and crate could too.

I initially had that thinking, too, which is why I had reached out to @pombredanne by chat to clarify. In that conversation he told me to think of the "package type" rather in terms of the "ecosystem", which includes protocols, formats and conventions.
It also helped me personally to further distinguish the "package type" from the "package format" (which is not directly covered in PURL, but could be via qualifiers). For Maven, the package type would be "Maven", but the package format would be "jar", "war", "aar" etc.

Here's my understanding from a purely lua point of view (anyone shout out if there is something I'm missing):

When making a new purl type, the vast majority seemed to be based around the packages that a package manager tool itself organizes (e.g. npm, cargo, maven) and...:

  • not the filename of a package in that ecosystem (gemfile, cargo.toml, go.mod)
  • not necessarily the name of the package manager tool (but sometimes it is)
  • not necessarily the name of the package (but sometimes it is)

There are some exceptions and inconsistencies:

  • when the package manager is baked into the language tooling -- for instance Go modules, the purl type is simply the language: golang.
  • the purl type for maven even though the tool on the CLI is mvn (you can argue they did that to make it easier to type, and that the name is really maven anyway).
  • if we were taking about "packages" then for rust crate would have been more accurate than cargo, but cargo had already been adopted unofficially so it stuck
  • ...there are more, but 🤷

</background thoughts>

@wagoodman
Copy link

wagoodman commented Feb 29, 2024

Some specific thoughts relating to this PR:

  • rockspec does not really fit as it's really only speaking to the file used to describe the larger logical rock packages
  • rock doesn't "seem" (hand wavy, I know) specific enough... but for some reason gem as a package type was good enough for ruby, instead of rubygem, so there is precedent for it as a candidate.
  • lua doesn't seem specific enough and refers to the language specifically. This was ok for golang as a type, but that's because the language canonically provided the method for distribution (go get). In rust's case they stuck with the candidate cargo (the tool) instead of crate (the logical package) because it was already a candidate in use (...and seemed close enough?).

So finally I think there are three candidates:

  • lua -- since it was already the candidate (but I don't really think it's a good fit)
  • rock -- since it is the name of the logical package (and there is precedence for this... this is my runner up)
  • luarock -- I like this one the best as it scopes the logical package to a language ecosystem.

@LaurentGoderre
Copy link
Author

My vote would be for luarock but I will wait until we have agreement to change it.

Signed-off-by: Laurent Goderre <[email protected]>
@LaurentGoderre
Copy link
Author

Updated.

Copy link

@wagoodman wagoodman left a comment

Choose a reason for hiding this comment

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

lots of thought to name things 😆 Thanks for the conversation on this 🙌

@wagoodman wagoodman merged commit 055233e into anchore:master Mar 12, 2024
3 checks passed
@pombredanne
Copy link

excellent... it would be great to also submit a PR for the spec to track this type and add tests entries so every tools can also use it.

@LaurentGoderre LaurentGoderre deleted the type-rockspec branch March 13, 2024 14:29
@LaurentGoderre
Copy link
Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants