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

fix: the package replace logic when using tag, branch or commit #2299

Merged
merged 10 commits into from
Mar 15, 2024

Conversation

leoporoli
Copy link
Contributor

@leoporoli leoporoli commented Mar 14, 2024

Description

Fix the package replace logic when using tag, branch or commit

The current issue is the package replace logic fails when you add a tag, branch or commit in the replace value, like this example:

replace: - github.com/kurtosis-tech/ethereum-package: github.com/kurtosis-tech/ethereum-package@da55be84861e93ce777076e545abee35ff2d51ce

If we see the evaluation error

Evaluation error: An error occurred while loading the module 'github.com/kurtosis-tech/ethereum-package@da55be84861e93ce777076e545abee35ff2d51ce/src/package_io/input_parser.star' Caused by: '/input_parser.star' doesn't exist in the package 'kurtosis-tech/ethereum-package' at [github.com/kurtosis-tech/ethereum-package@da55be84861e93ce777076e545abee35ff2d51ce/main.star:1:29]: <toplevel> at [github.com/kurtosis-tech/awesome-kurtosis/chainlink-node/main.star:2:33]: <toplevel>

we understand it's trying to import the file from this path github.com/kurtosis-tech/ethereum-package@da55be84861e93ce777076e545abee35ff2d51ce and it doesn't exist because the folder, inside the APIC file system, is created without the hash, it's github.com/kurtosis-tech/ethereum-package

This is happening because an absolute locator like this github.com/kurtosis-tech/ethereum-package/src/package_io/input_parser.star does not incorporate the concept of hash, branch or commit.

The fix is incorporated the concept of hash, branch or commit in a new AbsoluteLocator object, so this way we can specify if the absolute locator is pointing to other place than the main branch.

With this change we can accept the github.com/kurtosis-tech/ethereum-package@da55be84861e93ce777076e545abee35ff2d51ce replace value and an absolute locator string like this github.com/kurtosis-tech/ethereum-package@da55be84861e93ce777076e545abee35ff2d51ce/src/package_io/input_parser.star will be represented with an AbsoluteLocator object with locator github.com/kurtosis-tech/ethereum-package/src/package_io/input_parser.star and and tagBranchOrCommit da55be84861e93ce777076e545abee35ff2d51ce

We already had support for retrieving packages from different tag, branch or commit but only for packageId values and not for absolute locators

REMINDER: Tag Reviewers, so they get notified to review

Is this change user facing?

YES

References (if applicable)

Fix #2279

@leoporoli leoporoli closed this Mar 14, 2024
@kurtosis-tech kurtosis-tech deleted a comment from gitguardian bot Mar 14, 2024
@leoporoli leoporoli reopened this Mar 14, 2024
Copy link

gitguardian bot commented Mar 14, 2024

️✅ There are no secrets present in this pull request anymore.

If these secrets were true positive and are still valid, we highly recommend you to revoke them.
Once a secret has been leaked into a git repository, you should consider it compromised, even if it was deleted immediately.
Find here more information about risks.


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

Our GitHub checks need improvements? Share your feedbacks!

@tedim52
Copy link
Contributor

tedim52 commented Mar 15, 2024

approved! Is there any chance you can provide more context on what the specific bug around locators was and what the specific fix was for it was in the PR description?

@leoporoli
Copy link
Contributor Author

approved! Is there any chance you can provide more context on what the specific bug around locators was and what the specific fix was for it was in the PR description?

@tedim52 I added more context in the description, I hope this can put more light in the issue and the fix.

@tedim52
Copy link
Contributor

tedim52 commented Mar 15, 2024

Sweet, thanks for adding context to the PR, LGTM!

@leoporoli leoporoli added this pull request to the merge queue Mar 15, 2024
Merged via the queue into main with commit 72da1cb Mar 15, 2024
56 checks passed
@leoporoli leoporoli deleted the lporoli/fix-replace branch March 15, 2024 19:09
github-merge-queue bot pushed a commit that referenced this pull request Mar 18, 2024
🤖 I have created a release *beep* *boop*
---


##
[0.88.8](0.88.7...0.88.8)
(2024-03-15)


### Features

* Add active cancel at period end payment subscription status
([#2307](#2307))
([eea4e9b](eea4e9b))


### Bug Fixes

* the package replace logic when using tag, branch or commit
([#2299](#2299))
([72da1cb](72da1cb)),
closes [#2279](#2279)

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: kurtosisbot <kurtosisbot@users.noreply.github.com>
tedim52 pushed a commit that referenced this pull request Mar 21, 2024
## Description
Fix the package replace logic when using tag, branch or commit

The current issue is the `package replace logic` fails when you add a
tag, branch or commit in the replace value, like this example:

`replace:
- github.com/kurtosis-tech/ethereum-package:
github.com/kurtosis-tech/ethereum-package@da55be84861e93ce777076e545abee35ff2d51ce`


If we see the evaluation error 

`Evaluation error: An error occurred while loading the module
'github.com/kurtosis-tech/ethereum-package@da55be84861e93ce777076e545abee35ff2d51ce/src/package_io/input_parser.star'
Caused by: '/input_parser.star' doesn't exist in the package
'kurtosis-tech/ethereum-package'
at
[github.com/kurtosis-tech/ethereum-package@da55be84861e93ce777076e545abee35ff2d51ce/main.star:1:29]:
<toplevel>
at
[github.com/kurtosis-tech/awesome-kurtosis/chainlink-node/main.star:2:33]:
<toplevel>`

we understand it's trying to import the file from this path
`github.com/kurtosis-tech/ethereum-package@da55be84861e93ce777076e545abee35ff2d51ce`
and it doesn't exist because the folder, inside the APIC file system, is
created without the hash, it's
`github.com/kurtosis-tech/ethereum-package`

This is happening because an absolute locator like this
`github.com/kurtosis-tech/ethereum-package/src/package_io/input_parser.star`
does not incorporate the concept of hash, branch or commit.

The fix is incorporated the concept of `hash, branch or commit` in a new
AbsoluteLocator object, so this way we can specify if the absolute
locator is pointing to other place than the main branch.
            
With this change we can accept the
`github.com/kurtosis-tech/ethereum-package@da55be84861e93ce777076e545abee35ff2d51ce`
replace value and an absolute locator string like this
`github.com/kurtosis-tech/ethereum-package@da55be84861e93ce777076e545abee35ff2d51ce/src/package_io/input_parser.star`
will be represented with an AbsoluteLocator object with locator
`github.com/kurtosis-tech/ethereum-package/src/package_io/input_parser.star`
and and tagBranchOrCommit `da55be84861e93ce777076e545abee35ff2d51ce`

We already had support for retrieving packages from different tag,
branch or commit but only for `packageId` values and not for `absolute
locators`



## REMINDER: Tag Reviewers, so they get notified to review

## Is this change user facing?
YES

## References (if applicable)
Fix #2279
tedim52 pushed a commit that referenced this pull request Mar 21, 2024
🤖 I have created a release *beep* *boop*
---


##
[0.88.8](0.88.7...0.88.8)
(2024-03-15)


### Features

* Add active cancel at period end payment subscription status
([#2307](#2307))
([eea4e9b](5fbe401))


### Bug Fixes

* the package replace logic when using tag, branch or commit
([#2299](#2299))
([72da1cb](273861c)),
closes [#2279](#2279)

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: kurtosisbot <kurtosisbot@users.noreply.github.com>
tedim52 pushed a commit that referenced this pull request Mar 21, 2024
## Description
Fix the package replace logic when using tag, branch or commit

The current issue is the `package replace logic` fails when you add a
tag, branch or commit in the replace value, like this example:

`replace:
- github.com/kurtosis-tech/ethereum-package:
github.com/kurtosis-tech/ethereum-package@da55be84861e93ce777076e545abee35ff2d51ce`


If we see the evaluation error 

`Evaluation error: An error occurred while loading the module
'github.com/kurtosis-tech/ethereum-package@da55be84861e93ce777076e545abee35ff2d51ce/src/package_io/input_parser.star'
Caused by: '/input_parser.star' doesn't exist in the package
'kurtosis-tech/ethereum-package'
at
[github.com/kurtosis-tech/ethereum-package@da55be84861e93ce777076e545abee35ff2d51ce/main.star:1:29]:
<toplevel>
at
[github.com/kurtosis-tech/awesome-kurtosis/chainlink-node/main.star:2:33]:
<toplevel>`

we understand it's trying to import the file from this path
`github.com/kurtosis-tech/ethereum-package@da55be84861e93ce777076e545abee35ff2d51ce`
and it doesn't exist because the folder, inside the APIC file system, is
created without the hash, it's
`github.com/kurtosis-tech/ethereum-package`

This is happening because an absolute locator like this
`github.com/kurtosis-tech/ethereum-package/src/package_io/input_parser.star`
does not incorporate the concept of hash, branch or commit.

The fix is incorporated the concept of `hash, branch or commit` in a new
AbsoluteLocator object, so this way we can specify if the absolute
locator is pointing to other place than the main branch.
            
With this change we can accept the
`github.com/kurtosis-tech/ethereum-package@da55be84861e93ce777076e545abee35ff2d51ce`
replace value and an absolute locator string like this
`github.com/kurtosis-tech/ethereum-package@da55be84861e93ce777076e545abee35ff2d51ce/src/package_io/input_parser.star`
will be represented with an AbsoluteLocator object with locator
`github.com/kurtosis-tech/ethereum-package/src/package_io/input_parser.star`
and and tagBranchOrCommit `da55be84861e93ce777076e545abee35ff2d51ce`

We already had support for retrieving packages from different tag,
branch or commit but only for `packageId` values and not for `absolute
locators`



## REMINDER: Tag Reviewers, so they get notified to review

## Is this change user facing?
YES

## References (if applicable)
Fix #2279
tedim52 pushed a commit that referenced this pull request Mar 21, 2024
🤖 I have created a release *beep* *boop*
---


##
[0.88.8](0.88.7...0.88.8)
(2024-03-15)


### Features

* Add active cancel at period end payment subscription status
([#2307](#2307))
([eea4e9b](f54c2e8))


### Bug Fixes

* the package replace logic when using tag, branch or commit
([#2299](#2299))
([72da1cb](1abcdcc)),
closes [#2279](#2279)

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: kurtosisbot <kurtosisbot@users.noreply.github.com>
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.

fix bug in package replace that breaks versioned imports
2 participants