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

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

Open
proletarius101 opened this issue Jan 28, 2021 · 26 comments
Labels

Comments

@proletarius101
Copy link
Contributor

proletarius101 commented Jan 28, 2021

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

flathub/flathub#2086

@proletarius101 proletarius101 changed the title flatpak-node-generator.py failed to deal with deps with direct git reference [node] flatpak-node-generator.py failed to deal with deps with direct git reference Jan 28, 2021
@proletarius101
Copy link
Contributor Author

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/[email protected] from the tree as its been replaced by a newer version or is no longer required
23 silly removeObsoleteDep removing @standardnotes/[email protected] from the tree as its been replaced by a newer version or is no longer required
24 silly removeObsoleteDep removing [email protected] from the tree as its been replaced by a newer version or is no longer required
25 silly removeObsoleteDep removing [email protected] from the tree as its been replaced by a newer version or is no longer required
26 silly removeObsoleteDep removing @standardnotes/[email protected] from the tree as its been replaced by a newer version or is no longer required

@proletarius101
Copy link
Contributor Author

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/[email protected]. That's why it can't use the cache

@catsout
Copy link
Contributor

catsout commented Jan 29, 2021

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
Copy link
Contributor Author

proletarius101 commented Jan 29, 2021

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
Copy link
Contributor Author

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
Copy link
Contributor

catsout commented Jan 29, 2021

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

@proletarius101
Copy link
Contributor Author

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
Copy link
Contributor

catsout commented Jan 29, 2021

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
Copy link
Contributor Author

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
Copy link
Contributor Author

@catsout Thanks for pointing me to the right direction. standardnotes/app#516 fixes the corrupted submodules for this case.

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

@proletarius101
Copy link
Contributor Author

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://[email protected]/duosecurity/duo_web_sdk.git#410a9186cc34663c4913b17d6528067cd3331f1d doesn't match any Git prefix

@proletarius101
Copy link
Contributor Author

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
Copy link
Member

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

@proletarius101
Copy link
Contributor Author

@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://[email protected]/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
Copy link

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
Copy link
Member

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

@jwillikers
Copy link

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
Copy link
Member

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

@jwillikers
Copy link

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

@gasinvein
Copy link
Member

gasinvein commented Aug 14, 2021

@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
Copy link

jwillikers commented Aug 14, 2021

@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
Copy link
Member

@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
Copy link
Contributor Author

@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
Copy link

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
Copy link

PR: flathub/im.riot.Riot#200

@olof-nord
Copy link
Contributor

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://[email protected]/supershabam/nop.git"
  }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants