Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Update github hashes in npm package.json files #5640

Closed
piotr-s-brainhub opened this issue Mar 4, 2020 · 18 comments
Closed

Update github hashes in npm package.json files #5640

piotr-s-brainhub opened this issue Mar 4, 2020 · 18 comments

Comments

@piotr-s-brainhub
Copy link
Contributor

Sometimes I have dependencies in my package.json defined as a link to a git (GitHub, BitBucket or GitLab) repo, possibly with a branch name or a hash.

So it would be nice if renovate allowed me two options:

  • replace git dependencies into NPM dependencies
  • update git dependencies to the newest hash
@rarkins rarkins added manager:npm package.json files (npm/yarn/pnpm) type:feature Feature (new functionality) needs-requirements priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others labels Mar 4, 2020
@rarkins
Copy link
Collaborator

rarkins commented Mar 4, 2020

  • replace git dependencies into NPM dependencies

We don't plan to support this automatically, because there's too many ways such PRs could be wrong/unwelcome. We assume most people will use a GitHub dependency instead of npm for a good reason, and we may not know it.

  • update git dependencies to the newest hash

This is possible. Can you collect the possible syntaxes that would need to be supported?

@rarkins rarkins changed the title handling git dependencies Update github hashes in npm package.json files Mar 4, 2020
@piotr-s-brainhub
Copy link
Contributor Author

"@brainhubeu/gatsby-docs-kit": "RobertHebel/gatsby-docs-kit.git#upgrade-gatsby-to-v2",
"@brainhubeu/gatsby-docs-kit": "https://github.com/RobertHebel/gatsby-docs-kit.git#upgrade-gatsby-to-v2",
"react-native-camera": "git+https://git@github.com/react-native-community/react-native-camera",

at the begin:

  • git+https://git@github.com/
  • https://github.com/
  • nothing so it's GitHub as default

at the end:

  • hash
  • branch name
  • nothing so it fetches from the newest hash of the default branch

@rarkins
Copy link
Collaborator

rarkins commented Mar 4, 2020

How could Renovate know what to upgrade RobertHebel/gatsby-docs-kit.git#upgrade-gatsby-to-v2 to though? That seems like a very specific branch and not something to be updated. I had expected more like RobertHebel/gatsby-docs-kit.git#abcdef0 which then could be handled as so:

  • Check the latest commit on the default branch
  • If it's not abcdef0 then update it to the latest commit

Adding a commit hash to a URL that doesn't have one is a good idea, but definitely something we'd want people to opt into. We refer to updating digests as "digest updating" and adding digests as "digest pinning" btw.

@piotr-s-brainhub
Copy link
Contributor Author

it wasn't a good example, the hash should be updated only if there's already a hash

@rarkins
Copy link
Collaborator

rarkins commented Mar 4, 2020

Phase 1: If a hash exists then update it
Phase 2: If no hash or commitish reference exists but pinDigests is enabled, then add a commit hash

@m4theushw
Copy link

m4theushw commented Dec 27, 2021

I have this same need. In my case, I don't need support for commit hash but only the branch name at the end of the URL, which might not be the default one. I started to look on how to support this kind of dependency and we could reuse the git-refs datasource. The locked version set by the lock file should be bypassed or Renovate won't update. May I open a PR with an initial implementation of the support for Git URLs with a branch at the end?

@rarkins
Copy link
Collaborator

rarkins commented Dec 27, 2021

@m4theushw can you create a reproduction first and show manually what the PR should look like? Then I can advise if a PR would be a good idea.

@rarkins rarkins added the auto:reproduction A minimal reproduction is necessary to proceed label Dec 27, 2021
@github-actions
Copy link
Contributor

Hi there,

Help us by making a minimal reproduction repository.

Before we can start work on your issue we first need to know exactly what's causing the current behavior. A minimal reproduction helps us with this.

To get started, please read our guide on creating a minimal reproduction to understand what is needed.

We may close the issue if you (or someone else) have not provided a minimal reproduction within two weeks. If you need more time, or are stuck, please ask for help or more time in a comment.

Good luck,

The Renovate team

@m4theushw
Copy link

@rarkins Here's the reproduction: https://github.com/m4theushw/renovate-git-urls

The real application using this dependency is https://github.com/mui-org/material-ui-x/blob/master/package.json#L68

To add support for branch-based git urls, the first step would be to replace the skipReason below with the information for the dependency. The datasource would be set to git-refs. For the versioning, I'm not sure how far git could be reused. The versioning needed here is similar to docker.

dep.skipReason = SkipReason.UnversionedReference;

The next step is to make sure that the versioning.matches will return true when "branchname" is given to it. By default, the git versioning always returns false.

(v) => unconstrainedValue || versioning.matches(v.version, currentValue)

I stopped here but additional steps may be needed.

@rarkins
Copy link
Collaborator

rarkins commented Dec 28, 2021

Can you manually create the PR you expect? I'm not sure what you're expecting because it's set to a non-versioned branch name

@oliviertassinari
Copy link
Contributor

Can you manually create the PR you expect?

@rarkins This is the PR we would expect https://github.com/mui-org/material-ui-x/pull/3526/files.

How could Renovate know what to upgrade RobertHebel/gatsby-docs-kit.git#upgrade-gatsby-to-v2 to though?

I think that the answer on this one from @piotr-s-brainhub would be the latest git hash commit this branch refers to.

@viceice
Copy link
Member

viceice commented Dec 30, 2021

That's a lockfile only update. Use lockfile maintenance for this.

@m4theushw
Copy link

@viceice The dependency is not versioned for lockfile maintenance to work. There's a branch name at the end of the URL: https://github.com/RobertHebel/gatsby-docs-kit.git#upgrade-gatsby-to-v2. We need that Renovate upgrade the resolved value to be the last commit hash in the specified branch.

@viceice
Copy link
Member

viceice commented Dec 30, 2021

Lockfile maintenance should work, as renovate simply deletes the lockfile and let yarn re-create, so yarn should resolve to latest commit.

Renovate never directly changes the lockfile.

@m4theushw
Copy link

m4theushw commented Dec 30, 2021

I added a versioned dependency (lodash in this case) to my test repo and enabled lockfile maintenance. Renovate created two PRs: one to update lodash and another "Lock file maintenance" updating the resolved hash of the git url. The problem is that the lockfile maintenance PR is also updating lodash.

Is there a way to update via lockfile maintenance only those dependencies whose a dedicated PR can't be created? Sometimes the update of a single dependency needs intervention to fix the CI and handling all updates at once (in the lockfile maintenance PR) is not feasible.

@oliviertassinari
Copy link
Contributor

@m4theushw I approached it differently in mui/mui-x#3500

@m4theushw
Copy link

Any update here?

@rarkins rarkins added status:requirements Full requirements are not yet known, so implementation should not be started and removed status:ready labels Apr 21, 2023
@rarkins
Copy link
Collaborator

rarkins commented May 12, 2023

A new reproduction is needed here

@rarkins rarkins removed type:feature Feature (new functionality) priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others manager:npm package.json files (npm/yarn/pnpm) auto:reproduction A minimal reproduction is necessary to proceed status:requirements Full requirements are not yet known, so implementation should not be started labels Oct 1, 2023
@renovatebot renovatebot locked and limited conversation to collaborators Oct 1, 2023
@rarkins rarkins converted this issue into discussion #24771 Oct 1, 2023

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants