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: make pack and exec work with git hash refs #7815

Merged
merged 1 commit into from
Oct 14, 2024

Conversation

milaninfy
Copy link
Contributor

@milaninfy milaninfy commented Oct 4, 2024

Passing arborist constructor to pacote.manifest call because internally when fetching git deps DirFetcher requires Arborist constructor from GitFetcher https://github.com/npm/pacote/blob/main/CHANGELOG.md#1400-pre3-2022-09-28

  • Trying to add some tests

Fixes: #6723

@milaninfy milaninfy force-pushed the mm/fix-npx-git-long-sha branch from d79c6de to c629c52 Compare October 7, 2024 18:56
@milaninfy milaninfy marked this pull request as ready for review October 8, 2024 01:24
@milaninfy milaninfy requested a review from a team as a code owner October 8, 2024 01:24
@milaninfy milaninfy force-pushed the mm/fix-npx-git-long-sha branch from c629c52 to c68b9a0 Compare October 8, 2024 18:17
@milaninfy milaninfy force-pushed the mm/fix-npx-git-long-sha branch from c68b9a0 to 88615a2 Compare October 9, 2024 14:10
@wraithgar wraithgar self-assigned this Oct 9, 2024
@@ -7,6 +7,7 @@ const spawk = tspawk(t)

const fs = require('node:fs')
const path = require('node:path')
const { resolve } = require('node:path')
Copy link
Member

Choose a reason for hiding this comment

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

we're already requiring node:path in line 9.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll correct it as chore in next pr whenever I get chance.


t.test('packs from git spec', async t => {
const spec = 'test/test#111111aaaaaaaabbbbbbbbccccccdddddddeeeee'
const pkgPath = path.resolve(__dirname, '../../fixtures/git-test.tgz')
Copy link
Member

Choose a reason for hiding this comment

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

I'm not the biggest fan of having this tgz file in fixtures, but if I weigh it against the effort required to build it during tests it makes sense.

@wraithgar wraithgar changed the title fix: pass arborist constructor to manifest call fix: make pack and exec work with git hash refs Oct 14, 2024
@wraithgar wraithgar merged commit 7f541e8 into npm:latest Oct 14, 2024
56 checks passed
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.

[BUG] npm pack <git-dependency>#<commit-hash> fails in npm >= 9.6.5
2 participants