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

feat(dart): use first version of constraint for dependencies using SDK version #6239

Merged

Conversation

DmitriyLewen
Copy link
Contributor

@DmitriyLewen DmitriyLewen commented Mar 1, 2024

Description

Use first version of constraint for dependencies using SDK version.
e.g. in this case the version of flutter_test should be 3.3.0:

  flutter_test:
    dependency: "direct dev"
    description: flutter
    source: sdk
    version: "0.0.0"
sdks:
  dart: ">=2.18.0 <3.0.0"
  flutter: "^3.3.0"

More information in #5984

Related issues

Checklist

  • I've read the guidelines for contributing to this repository.
  • I've followed the conventions in the PR title.
  • I've added tests that prove my fix is effective or that my feature works.
  • I've updated the documentation with the relevant information (if needed).
  • I've added usage information (if the PR introduces new options)
  • I've included a "before" and "after" example to the description (if the PR is a user interface change).

@DmitriyLewen DmitriyLewen self-assigned this Mar 1, 2024
@DmitriyLewen DmitriyLewen marked this pull request as ready for review March 1, 2024 07:33
@DmitriyLewen DmitriyLewen requested a review from knqyf263 as a code owner March 1, 2024 07:33
Copy link

This PR is stale because it has been labeled with inactivity.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and will be auto-closed. label May 26, 2024
Comment on lines 95 to 107
// firstVersionOfConstrain returns the first acceptable version for constraint
func firstVersionOfConstrain(constraint string) string {
// cf. https://dart.dev/tools/pub/dependencies#traditional-syntax
switch {
case strings.HasPrefix(constraint, ">="):
constraint = strings.TrimPrefix(constraint, ">=")
constraint, _, _ = strings.Cut(constraint, " ")
return constraint
case strings.HasPrefix(constraint, "^"):
return strings.TrimPrefix(constraint, "^")
}
return ""
}
Copy link
Collaborator

@knqyf263 knqyf263 May 30, 2024

Choose a reason for hiding this comment

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

I think we want to parse the constraints in a more stable way. What if we export constraint and add methods like func (c *Constraint) Version() string and func (c *Constraint) Operator() string in go-version?
https://github.com/aquasecurity/go-version/blob/637058cfe4921e395b56f195224538d7eac61520/pkg/version/constraint.go#L55-L59

type Constraint struct {
	version  Version
	operator string // e.g. "=", ">=", "^"
	operatorFunc operatorFunc
	original string
}

func (c *Constraint) Version() string {
    return c.version
}

func (c *Constraint) Operator() string {
    return c.operator
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea!
Created aquasecurity/go-version#6
And updated this PR - 0bbe7c5

Copy link
Collaborator

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.

Updated this PR -91c2b1b

@github-actions github-actions bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and will be auto-closed. label May 31, 2024
@DmitriyLewen DmitriyLewen force-pushed the feat-dart/sdk-versions-for-deps branch from f70d7ac to eb67467 Compare June 3, 2024 08:32
// cf. https://dart.dev/tools/pub/dependencies#traditional-syntax
constraints := css.List()
// We only need to get the first version from the range
if constraints[0][0].Operator() != ">=" && constraints[0][0].Operator() != "^" {
Copy link
Collaborator

@knqyf263 knqyf263 Jun 4, 2024

Choose a reason for hiding this comment

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

Can we check the length of constratints so it won't panic? I presume css.List() doesn't return empty, but unexpected things always happen. It's better to ensure it's non-empty.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it won't hurt.
Added check in 35a6f0a

DmitriyLewen and others added 2 commits June 4, 2024 11:30
if constraint, ok := l.Sdks[string(dep.Description)]; ok {
v, err := firstVersionOfConstrain(constraint)
if err != nil {
p.logger.Warn("unable to get sdk version from constraint: %w", log.Err(err))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
p.logger.Warn("unable to get sdk version from constraint: %w", log.Err(err))
p.logger.Warn("Unable to get sdk version from constraint", log.Err(err))

Copy link
Collaborator

Choose a reason for hiding this comment

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

Fixed in 191270c

@knqyf263 knqyf263 enabled auto-merge June 5, 2024 06:30
@knqyf263 knqyf263 added this pull request to the merge queue Jun 5, 2024
Merged via the queue into aquasecurity:main with commit 042d6b0 Jun 5, 2024
17 checks passed
@aqua-bot aqua-bot mentioned this pull request Jun 5, 2024
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.

feat(dart): use minimum SDK version for packages that use SDK for package version
2 participants