search

flatpak / flatpak-builder-tools

Various helper tools for flatpak-builder
197 stars 107 forks source link

[node] flatpak-node-generator.py failed to deal with deps with direct git reference #182

Open proletarius101 opened 3 years ago

proletarius101 commented 3 years ago

What is a dependency with direct git reference?

https://github.com/standardnotes/desktop/blob/3fbb58adf16a564d6773aae6cce273325f9eeba8/package.json#L12


  "dependencies": {
    ...
    "decrypt": "github:standardnotes/decrypt#master",
    ...
  },

What did I do?

python flatpak-node-generator.py npm ../standardnotes-desktop/package-lock.json --xdg-layout  -r

The results

npm ERR! code ENOTCACHED
npm ERR! request to https://registry.npmjs.org/@standardnotes%2fsncrypto-web failed: cache mode is 'only-if-cached' but no cached response available.

A complete log could be found at https://flathub.org/builds/#/builders/35/builds/4478

What I can suggest

crypto-web is a dependency of github:standardnotes/decrypt#master. I think it's failed to be included in the cache.

Related PR

https://github.com/flathub/flathub/pull/2086

proletarius101 commented 3 years ago

Some interesting logs:


21 silly removeObsoleteDep removing decrypt@github:standardnotes/decrypt#5d487038d3568d180e7cf561d774e1b685985d62 from the tree as its been replaced by a newer version or is no longer required
22 silly removeObsoleteDep removing @standardnotes/sncrypto-web@1.2.9 from the tree as its been replaced by a newer version or is no longer required
23 silly removeObsoleteDep removing @standardnotes/sncrypto-common@1.2.9 from the tree as its been replaced by a newer version or is no longer required
24 silly removeObsoleteDep removing libsodium-wrappers@0.7.8 from the tree as its been replaced by a newer version or is no longer required
25 silly removeObsoleteDep removing libsodium@0.7.8 from the tree as its been replaced by a newer version or is no longer required
26 silly removeObsoleteDep removing @standardnotes/snjs@2.0.22 from the tree as its been replaced by a newer version or is no longer required
proletarius101 commented 3 years ago

So after further investigation, a more elaborated reason is, the git-hosted module has a version, which is obvious, however, it's not tagged. npm tries to replace it with @standardnotes/sncrypto-web@1.2.9. That's why it can't use the cache

catsout commented 3 years ago

First you should use git source instead of tar.gz, as it has submodules. For npm git dependence try this, I try to fix it and not merged to here now.

You may need add env npm_config_offline: 'true' and npm_config_no_save: 'true', --no-save for check package{-lock}.json file in build dir when failed.

proletarius101 commented 3 years ago

First you should use git source instead of tar.gz, as it has submodules.

If I

      - type: git
        url: https://github.com/standardnotes/desktop
        tag: v3.5.18

Then it will 

FB: Running: git fetch -p --no-recurse-submodules --depth=1 -f origin '+refs/tags/3.5.17:refs/tags/3.5.17'
remote: Enumerating objects: 271, done.
remote: Counting objects: 100% (271/271), done.
remote: Compressing objects: 100% (251/251), done.
remote: Total 271 (delta 10), reused 132 (delta 5), pack-reused 0
Receiving objects: 100% (271/271), 735.50 KiB | 799.00 KiB/s, done.
Resolving deltas: 100% (10/10), done.
From https://github.com/standardnotes/web
 * [new tag]         3.5.17     -> 3.5.17
FB: Running: git rev-parse 687b53263f379466294a320e62204bfcada997cb
FB: Running: git rev-parse --verify --quiet 687b53263f379466294a320e62204bfcada997cb:.gitmodules
FB: Running: git show 687b53263f379466294a320e62204bfcada997cb:.gitmodules
FB: Running: git ls-tree 687b53263f379466294a320e62204bfcada997cb vendor/extensions/extensions-manager
Failed to download sources: module standardnotes: Not a gitlink tree: vendor/extensions/extensions-manager
proletarius101 commented 3 years ago

You need a proxy if you are in China, and refresh you proxy tool,dns cache then try more times.

No I'm not. And sorry I pasted the wrong log. Edited

catsout commented 3 years ago

Seems no way to fix, you may need add source manually.

proletarius101 commented 3 years ago

Seems no way to fix, you may need add source manually.

That, however, is probably a bug of flatpak-builder. standardnotes repo uses a nested submodule structure, while flatpak builder tries to git ls-tree submodules of the module web in the parent dir, which obvious is incorrect.

catsout commented 3 years ago

That, however, is probably a bug of flatpak-builder. standardnotes repo uses a nested submodule structure, while flatpak builder tries to git ls-tree submodules of the module web in the parent dir, which obvious is incorrect.

Maybe not this, as directly using standardnote/web's git source get the same error. It's seems that standardnote/web has .gitmodules but deleted the gitlink entries, which cause this error.

proletarius101 commented 3 years ago

That, however, is probably a bug of flatpak-builder. standardnotes repo uses a nested submodule structure, while flatpak builder tries to git ls-tree submodules of the module web in the parent dir, which obvious is incorrect.

Maybe not this, as directly using standardnote/web's git source get the same error. It's seems that standardnote/web has .gitmodules but deleted the gitlink entries, which cause this error.

That's interesting... It doesn't even clone the vendor/extensions/extensions-manager in vendor/extensions/extensions-manager!

proletarius101 commented 3 years ago

@catsout Thanks for pointing me to the right direction. https://github.com/standardnotes/web/pull/516 fixes the corrupted submodules for this case.

On the other hand, looking forward to your PR being merged ;)

proletarius101 commented 3 years ago

So, there is another case of git-based dependencies not covered. e.g. https://github.com/bitwarden/jslib/blob/859f317d59189d223072a406bc2d6924e1fb71bc/package-lock.json#L3262, there's a git+ssh source which is valid according to the doc. But the generator raises an error:

AssertionError: git+ssh://git@github.com/duosecurity/duo_web_sdk.git#410a9186cc34663c4913b17d6528067cd3331f1d doesn't match any Git prefix
proletarius101 commented 3 years ago

I found that the git+ssh source is explicitly disabled in the old npm generator. Is there any particular reason that we do this? And the new generator doesn't include git+ssh either? @gasinvein

Maybe it can't be simply plugged-in by adding one item in GIT_SCHEMES?

gasinvein commented 3 years ago

@proletarius101 Well, try adding entry 'git+ssh': {'scheme': 'git'} to GIT_SCHEMES and tell us if it helps in your case.

proletarius101 commented 3 years ago

@proletarius101 Well, try adding entry 'git+ssh': {'scheme': 'git'} to GIT_SCHEMES and tell us if it helps in your case.

It doesn't just work, but so close.

What it does

// package-lock.json
// From
    "duo_web_sdk": {
      "version": "git+ssh://git@github.com/duosecurity/duo_web_sdk.git#410a9186cc34663c4913b17d6528067cd3331f1d",
      "from": "duo_web_sdk@git+https://github.com/duosecurity/duo_web_sdk.git"
    },
// To
    "duo_web_sdk": {
      "version": "git+file:/******/flatpak-node/git-packages/duo_web_sdk-410a9186cc34663c4913b17d6528067cd3331f1d#410a9186cc34663c4913b17d6528067cd3331f1d",
      "from": "git+file:/******/flatpak-node/git-packages/duo_web_sdk-410a9186cc34663c4913b17d6528067cd3331f1d#410a9186cc34663c4913b17d6528067cd3331f1d"
    },

What it doesn't

It should have changed this as well 

// package.json
    "duo_web_sdk": "git+https://github.com/duosecurity/duo_web_sdk.git",
jwillikers commented 3 years ago

I'm hitting this same issue attempting to package Element from source, which uses a couple of direct Git references.

error Can't make a request in offline mode ("https://registry.yarnpkg.com/matrix-react-sdk/-/matrix-react-sdk-3.27.0.tgz")
gasinvein commented 3 years ago

@jwillikers Well, this particular package isn't a git one, so I doubt it's the same issue.

jwillikers commented 3 years ago

It's defined in the package.json using a direct Git reference:

"matrix-react-sdk": "github:matrix-org/matrix-react-sdk#develop",

However, it appears that matrix-web doesn't use Git references in their tagged releases, so hopefully I can sidestep the problem for now.

gasinvein commented 3 years ago

@jwillikers So where did the error you posted came from, from a tagged release or from main branch tip?

jwillikers commented 3 years ago

@gasinvein The error I posted above is from the tip of the default branch, develop, from element-web.

gasinvein commented 3 years ago

@jwillikers Why does it point to registry.yarnpkg.com, then? There is only one matrix-react-sdk in the lockfile at the same commit, and the lockfile entry points to github. Maybe you did generate sources for the wrong app version?

jwillikers commented 3 years ago

@gasinvein Well, from what I can tell, the generated-sources.json uses a dest-filename of a particular hash for matrix-react-sdk (which is downloaded in the cache as such), but then yarn appears to still try and download it during the build stage.

gasinvein commented 3 years ago

@jwillikers Can you please link to the manifest you're trying to build? It's hard to tell what's wrong without knowing what's going on.

proletarius101 commented 3 years ago

@gasinvein Well, from what I can tell, the generated-sources.json uses a dest-filename of a particular hash for matrix-react-sdk (which is downloaded in the cache as such), but then yarn appears to still try and download it during the build stage.

Did you miss the --xdg-layout flag?

jwillikers commented 3 years ago

Nope, I just regenerated the file just now using python3 flatpak-node-generator.py --electron-node-headers --xdg-layout -o generated-sources-web.json yarn yarn.lock

{
        "type": "file",
        "url": "https://codeload.github.com/matrix-org/matrix-react-sdk/tar.gz/e2ef5d1737acda03c186560182fa7daddff8479b",
        "sha256": "609eae2093a7823664b15f95ba3bd478bd85e1bb4ab5d0a8e7d08bf9324c70c5",
        "dest-filename": "e2ef5d1737acda03c186560182fa7daddff8479b",
        "dest": "flatpak-node/yarn-mirror"
    }

@gasinvein I'll go ahead an open a draft PR and link it here. Using the packages.json from the commit tagged for the latest release, I get around this issue... and now I'm on to another...

jwillikers commented 3 years ago

PR: flathub/im.riot.Riot#200

olof-nord commented 2 years ago

From what I can tell, the flatpak-builder-tools node tooling supports a subset of the ways npm can resolve a dependency.

Looking at a recent test case, the following should work: (see https://github.com/flatpak/flatpak-builder-tools/blob/master/node/tests/data/packages/minimal-git/package.json)

{
  "name": "@flatpak-node-generator-tests/minimal-git",
  "version": "1.0.0",
  "dependencies": {
    "nop": "https://git@github.com/supershabam/nop.git"
  }
}