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

Restore presence on AMO #679

Closed
lidel opened this issue Feb 12, 2019 · 4 comments · Fixed by #680
Closed

Restore presence on AMO #679

lidel opened this issue Feb 12, 2019 · 4 comments · Fixed by #680
Assignees
Labels
kind/maintenance Work required to avoid breaking changes or harm to project's status quo P0 Critical: Tackled by core team ASAP

Comments

@lidel
Copy link
Member

lidel commented Feb 12, 2019

IPFS Companion got temporarily hidden from addons.mozilla.org due to manual review.
We are working on complying with requests and suggestions from AMO reviewers.

Updates will be published below.

cc #306

Timeline (continued in comments)

2019-01-31

Manual review started. Reviewer had a build issue, we replied on the same day providing build steps:

[ Click to expand conversation ]

This version contains obfuscated, minified, concatenated or otherwise machine-generated code. Please provide the original sources, together with instructions on how to generate the final XPI. Source code must be provided as an archive and uploaded using the source code upload field, which can be done during submission or on the version page in the developer hub.
Please read through the instructions at https://developer.mozilla.org/en-US/Add-ons/Source_Code_Submission.

Are you able to provide more details? Which specific part (files, paths) of extension triggered this action?

We had similar request in past (for v2.4.2) and since then we always attach .zip with sources for every release and did the same for this one. The file included README with reproducible build instructions (copy below).

Reproducible build (requires Docker): npm run release-build
Regular build (no Docker): npm run dev-build

"release-build" is an alias for running build script inside of an immutable Docker image with specific version of Node, yarn and crypto toolkit, which guarantees the same node version and minified output on all platforms.

Related docs:
https://github.com/ipfs-shipyard/ipfs-companion#development
https://github.com/ipfs-shipyard/ipfs-companion#reproducible-build-in-docker
#306 (reproducible build issue – please comment if we do something wrong or could do better in this area)

2019-01-01

Reply from Reviewer suggesting the issue is resolved

  • Thank you for the prompt response. It seems I was looking at an older version. The source code for this one was provided correctly.

2019-01-01

Reply from Reviewer suggesting the webui is still missing
Replied on the same day:

[ Click to expand conversation ]

I've built your add-on on Ubuntu 18.04 using npm i, npm run release-build, but webui folder is missing from the build.

Interesting! Current build pipeline downloads prebuilt webui (sources at https://github.com/ipfs-shipyard/ipfs-webui) from IPFS instead of NPM. I assume your VM did not have IPFS installed, but it should fallback to fetching dependency from public HTTP gateway. Did your VM block HTTP GET from: https://ipfs.io/api/v0/get?arg=QmXc9raDM1M5G5fpBnVyQ71vR4gbnskwnB9iMEzBuLgvoZ&archive=true&compress=true ? Are you able to retry?

2019-02-08

Reply from Reviewer being unable to reproduce:

I tried it again but it got stuck at:

$ ./ci/update-manifest.sh ; npx [email protected] build ; chmod -R ugo+rwX build/ add-on/

fatal: Not a git repository (or any of the parent directories): .git

Mentioned error should not break the build and in the following two days we were trying to reproduce issues with the build locally without any success.

2019-02-10

Extension hidden due to reproducibility issues:

This version didn't pass review because of the following problems:
1) We couldn't reproduce the build.
Please submit a new version and provide all requested information at your convenience.

Due to the lack of better ideas I tracked down the issue with missing ".git" in .zip file and added a fix to make 'npm run release-build' work again without that error message.

Uploaded new version (v2.7.2) - ready for review again.

2019-02-11

Reviewer is still unable to reproduce and sends build log:

I tried it again on Ubuntu 18.04 and ran npm run release-build, but I'm getting some errors. I'm pasting here the build log:

[ Click to expand build log ]
npm run release-build



> ipfs-companion@ release-build /home/ralli/Downloads/ipfs-companion-2.7.2-src/ipfs-companion-2.7.2

> docker build -t ipfs-companion . && docker run -it -e RELEASE_CHANNEL=stable -v $(pwd)/build:/usr/src/app/build ipfs-companion yarn ci:build



Sending build context to Docker daemon  2.593MB

Step 1/9 : FROM node:10.11.0

 ---> 8672b25e842c

Step 2/9 : RUN mkdir -p /usr/src/app

 ---> Using cache

 ---> 4786f66115c5

Step 3/9 : WORKDIR /usr/src/app

 ---> Using cache

 ---> cd7705735ebc

Step 4/9 : ENV PATH="/usr/src/app/node_modules/.bin:${PATH}"

 ---> Using cache

 ---> bec1a88c69a7

Step 5/9 : RUN curl -s https://ipfs.io/ipfs/QmbukYcmtyU6ZEKt6fepnvrTNa9F6VqsUPMUgNxQjEmphH > /usr/local/bin/jq && chmod +x /usr/local/bin/jq

 ---> Using cache

 ---> 008ebd78a635

Step 6/9 : COPY package.json /usr/src/app/package.json

 ---> b15bbf53362c

Step 7/9 : COPY yarn.lock /usr/src/app/yarn.lock

 ---> 6cc27c19c99a

Step 8/9 : RUN npm run ci:install

 ---> Running in ed0d901155c0



> ipfs-companion@ ci:install /usr/src/app

> npx [email protected] install --frozen-lockfile || npx [email protected] install --frozen-lockfile



npx: installed 1 in 4.532s

yarn install v1.12.3

[1/4] Resolving packages...

warning Resolution field "[email protected]" is incompatible with requested version "multiaddr@^5.0.2"

warning Resolution field "[email protected]" is incompatible with requested version "multiaddr@^5.0.2"

warning Resolution field "[email protected]" is incompatible with requested version "multiaddr@^5.0.0"

warning Resolution field "[email protected]" is incompatible with requested version "multiaddr@^5.0.2"

warning Resolution field "[email protected]" is incompatible with requested version "multiaddr@^5.0.0"

warning Resolution field "[email protected]" is incompatible with requested version "multiaddr@^5.0.2"

warning Resolution field "[email protected]" is incompatible with requested version "multiaddr@^4.0.0"

warning Resolution field "[email protected]" is incompatible with requested version "multiaddr@^5.0.2"

warning Resolution field "[email protected]" is incompatible with requested version "multiaddr@^5.0.0"

warning Resolution field "[email protected]" is incompatible with requested version "stream-http@^2.7.2"

[2/4] Fetching packages...

Removing intermediate container ed0d901155c0

 ---> dea203892a52

Step 9/9 : COPY . /usr/src/app

 ---> 4ad5bec12418

Successfully built 4ad5bec12418

Successfully tagged ipfs-companion:latest

yarn run v1.9.4

$ ./ci/update-manifest.sh ; npx [email protected] build ; chmod -R ugo+rwX build/ add-on/

Skipping manifest modification (RELEASE_CHANNEL=stable)

npx: installed 1 in 2.014s

$ run-s clean build:*

/bin/sh: 1: run-s: not found

error Command failed with exit code 127.

Please test your build steps on a virtual machine with a clean setup and update the instructions for us.

Turns out the build process was trying to fetch packages from registry.js.ipfs.io and failed silently because server was down. It worked on our end because we had cached docker image built when server was online.

I reported issue to our internal infra team but to save time switched to registry.yarnpkg.com (bf9a69e) and made a new release (v2.7.3) and asked for a fresh look.


(continued in comments)

@lidel lidel added the kind/maintenance Work required to avoid breaking changes or harm to project's status quo label Feb 12, 2019
@lidel lidel self-assigned this Feb 12, 2019
@lidel lidel added P0 Critical: Tackled by core team ASAP status/in-progress In progress labels Feb 12, 2019
@lidel
Copy link
Member Author

lidel commented Feb 12, 2019

2019-02-12

(review of v2.7.3)

The build issue seems to be resolved on Reviewer's machine (:tada:), but Web UI itself becomes an issue:

From what I'm see the webui code is downloaded from https://ipfs.io/api and appended to the build. Please provide the original source code for it and attach it to this version on AMO.

Our reply:

Web UI is just a dependency, like NPM ones.

The only difference is that instead of fetching it from NPM servers it is fetched from IPFS.

Sources of used webui can be found at: https://github.com/ipfs-shipyard/ipfs-webui/releases/tag/v2.3.3

Let us know if that is enough to move forward, or what are required steps to restore extension's availability in AMO.

There are multiple dependencies fetched from NPM and we are not being asked to attach sources of them. How can we improve things for dependencies fetched from IPFS to be equally "valid"?

(waiting)

@lidel lidel pinned this issue Feb 12, 2019
@lidel
Copy link
Member Author

lidel commented Feb 13, 2019

2019-02-13

Reached out on #addon-reviewers @ irc.mozilla.org to clarify any issues around what our extension does. Around the same time got review feedback regarding webui:

  1. To answer your question: if you have private packages you can upload them along with the rest of the source code, so we can review them.

  2. I've built webui branch https://github.com/ipfs-shipyard/ipfs-webui/releases/tag/v2.3.3 using node 8.12.0 and npm 6.4.1, but the resulting code is different from the webui folder in the xpi. Please see the attached build https://drive.google.com/open?id=1Ba9kvMhSUZM0KMQMhP7ebYmGX8CCAUAh
    Please test your build steps on a virtual machine with a clean setup and update the instructions for us.

I created an upstream issue to resolve reproducibility of ipfs-webui ipfs/ipfs-webui#959

To solve the issue at hand, I will prepare a quick bugfix release that does not bundle webui and opens it from local gateway instead.

@lidel
Copy link
Member Author

lidel commented Feb 15, 2019

2019-02-13

Spent a lot of time debugging differences between Firefox and Chrome, namely handling CORS in webRequest API. In the end, managed to get it work in both browsers and removed webui in #680.

Companion is now able to load Web UI from IPFS (it's a feature!) and ensure it can access API set in Companion's Preferences screen without any additional configuration of go-ipfs (seamless user experience 👌)

2019-02-14

Submitted v2.7.4 to http://addons.mozilla.org, waiting for review 🤞

@lidel
Copy link
Member Author

lidel commented Feb 15, 2019

2019-02-15

We are back in business!

AMO asked us to provide Privacy Policy. I published a draft here and will track it in #681.

@lidel lidel closed this as completed Feb 15, 2019
@ghost ghost removed the status/in-progress In progress label Feb 15, 2019
lidel added a commit that referenced this issue Feb 20, 2019
This is initial version published to AMO on 2019-02-15
Bare minimum, just to pass the review (#679)
@lidel lidel unpinned this issue Feb 21, 2019
lidel added a commit that referenced this issue Mar 7, 2019
This is cleanup related to #679

We no longer bundle webui, and there is a better alternative:
https://github.com/hacdias/ipfs-or-gateway
lidel added a commit that referenced this issue Mar 13, 2019
This is cleanup related to #679

We no longer bundle webui, and there is a better alternative:
https://github.com/hacdias/ipfs-or-gateway
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/maintenance Work required to avoid breaking changes or harm to project's status quo P0 Critical: Tackled by core team ASAP
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant