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

Desktop build is not updated #7987

Closed
mvtglobally opened this issue Mar 3, 2022 · 29 comments
Closed

Desktop build is not updated #7987

mvtglobally opened this issue Mar 3, 2022 · 29 comments
Assignees
Labels
DeployBlockerCash This issue or pull request should block deployment Engineering External Added to denote the issue can be worked on by a contributor Hourly KSv2

Comments

@mvtglobally
Copy link

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Action Performed:

  1. keep previous build app open in docket
  2. wait for next build update notification

Expected Result:

1.1.41-0 build should be either autoupdated or user should get notification

Actual Result:

Build is not updated. Desktop staging app is still 1.1.40-2

Context
This is potentially because of #7744. It might also just be because of #7765, which increase the interval at which NewDot check for updates from 1 hour to 8 hours.
The Desktop app output is no longer in dist but in desktop-build
Otherwise we're now build for 3 target x86, arm64 and universal apps, could that be a problem? We used to build only x86 before.
More info here #7954 (comment)

Workaround:

unknown

Platform:

Where is this issue occurring?

  • Web
  • iOS
  • Android
  • Desktop App
  • Mobile Web

Version Number: 1.1.40-2
Reproducible in staging?: Y
Reproducible in production?: N
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
Screen Shot 2022-03-02 at 9 53 23 AM

Expensify/Expensify Issue URL:
Issue reported by: Applause
Slack conversation:

View all open jobs on GitHub

@MelvinBot
Copy link

Triggered auto assignment to @flodnv (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@mvtglobally mvtglobally added the DeployBlockerCash This issue or pull request should block deployment label Mar 3, 2022
@github-actions github-actions bot added Hourly KSv2 and removed Daily KSv2 labels Mar 3, 2022
@OSBotify
Copy link
Contributor

OSBotify commented Mar 3, 2022

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open StagingDeployCash deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

@flodnv flodnv removed their assignment Mar 3, 2022
@flodnv flodnv added the External Added to denote the issue can be worked on by a contributor label Mar 3, 2022
@MelvinBot
Copy link

Triggered auto assignment to @michaelhaxhiu (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

@MelvinBot MelvinBot added Daily KSv2 and removed Hourly KSv2 labels Mar 3, 2022
@flodnv
Copy link
Contributor

flodnv commented Mar 3, 2022

@kidroca can you take this given you've already done part of the investigation?

@kidroca
Copy link
Contributor

kidroca commented Mar 3, 2022

Ok, can we start with someone answering my questions here : #7954 (comment)

  • What happens after Desktop is built?
  • Is there a way to manually download Desktop?
  • Could it be the lack of this zip?

Is the guide here meant for external contributors?
https://github.com/Expensify/App/blob/main/desktop/README.md#testing-electron-auto-update

I'll investigate and report back

@flodnv
Copy link
Contributor

flodnv commented Mar 3, 2022

@neil-marcellini @roryabraham can you help out @kidroca here?

Is there a way to manually download Desktop?

Yes, here: https://new.expensify.com/NewExpensify.dmg and here https://staging.new.expensify.com/NewExpensify.dmg AFAIK

@kidroca

This comment was marked as outdated.

@kidroca
Copy link
Contributor

kidroca commented Mar 3, 2022

Is there a way to manually download Desktop?

Yes, here: https://new.expensify.com/NewExpensify.dmg and here https://staging.new.expensify.com/NewExpensify.dmg AFAIK

What is making it available there though? Right now I downloaded v1.1.40-2 from that link while v1.1.41-0 seems to be the latest version

What would put a new version at that route?
A .dmg search review only 3 results in the repo: https://github.com/Expensify/App/search?q=.dmg&type=code

@kidroca

This comment was marked as outdated.

@kidroca
Copy link
Contributor

kidroca commented Mar 3, 2022

Oh wow, it seems to be caused by removing the zip file indeed: electron-userland/electron-builder#2199 (comment)

@kidroca
Copy link
Contributor

kidroca commented Mar 3, 2022

Submitted a PR that should fix auto-updates: #7989

I'm not sure whether it would fix the .dmg being hosted here: https://new.expensify.com/NewExpensify.dmg

@roryabraham
Copy link
Contributor

Self-assigning and trying to get a fix ASAP.

@roryabraham
Copy link
Contributor

Looking at the deploy logs from #7989, I'm seeing some suspicious log lines:

uploading       file=New Expensify-1.1.41-2.dmg
...
uploading       file=New Expensify-1.1.41-2-arm64.dmg provider=S3
...
uploading       file=New Expensify-1.1.41-2-universal.dmg provider=S3

Going to investigate whether the package version being tacked onto the file is our problem. Also @kidroca I don't recall why we added target architectures in electronBuilder.config.js 🤔

@roryabraham
Copy link
Contributor

Older successful deploys had this:

uploading       file=NewExpensify.dmg provider=S3

Noticing that there was:

  1. no version in the filename
  2. No space between New and Expensify.

@roryabraham
Copy link
Contributor

I'm hopeful that I've figured this out ... just testing with local builds which takes a lil while

@kidroca
Copy link
Contributor

kidroca commented Mar 4, 2022

Also @kidroca I don't recall why we added target architectures in electronBuilder.config.js 🤔

So that we can support new M1 macs without them running App through Rosetta
Though if arm64 or the universal app are not exposed for downloading it's probably no good

@roryabraham
Copy link
Contributor

Though if arm64 or the universal app are not exposed for downloading it's probably no good

I was wondering if a single .dmg could/would be "smart enough" to detect the architecture and install the correct app 🤔

@botify botify closed this as completed Mar 4, 2022
@kidroca
Copy link
Contributor

kidroca commented Mar 4, 2022

I was wondering if a single .dmg could/would be "smart enough" to detect the architecture and install the correct app

Only if we make a custom one - the ones from electron-builder are .dmg per arch
The universal works for bot but it's twice the size (installer and installed), though it's still 2x smaller than what we had before

We decided to not to make custom installer but make downloading download the correct .dmg for the user #6927 (comment)

@roryabraham roryabraham reopened this Mar 4, 2022
@roryabraham
Copy link
Contributor

Unfortunately I don't think this is resolved yet. #7996 changed these suspicious logs: https://github.com/Expensify/App/runs/5416585778?check_suite_focus=true

But I downloaded the staging desktop app and it's still on version 1.1.40-2

@kidroca
Copy link
Contributor

kidroca commented Mar 4, 2022

Interesting how @mallenexpensify updated to v1.1.41-2 here: #7579 (comment)?

@roryabraham
Copy link
Contributor

Le whoops ... staging builds are deploying the app straight to the production bucket 😬

@roryabraham
Copy link
Contributor

I downloaded the app from https://new.expensify.com/NewExpensify.dmg and it was 1.1.41-3 😬

@roryabraham
Copy link
Contributor

Fortunately this is only affecting desktop and not web

@kidroca
Copy link
Contributor

kidroca commented Mar 4, 2022

Sorry, seems this has to have export so that it works in the config as well

ELECTRON_ENV=${1:-development}

export ELECTRON_ENV=${1:-development}

we use it for the isStaging check

@roryabraham
Copy link
Contributor

Yep, that's probably it. I'll work on a PR

@kidroca
Copy link
Contributor

kidroca commented Mar 4, 2022

Hey BTW these work as well

We can use something like

artifactName: 'NewExpensify-{arch}.dmg',

to have platform specific .dmgs

@roryabraham
Copy link
Contributor

roryabraham commented Mar 4, 2022

artifactName: 'NewExpensify-{arch}.dmg',

That seems like a good idea. I'm going to create a follow-up issue to address the multiple architectures.

@kidroca
Copy link
Contributor

kidroca commented Mar 4, 2022

https://www.electron.build/file-patterns#file-macros

You can use macros in the file patterns, artifact file name patterns and publish configuration url:

${arch} — expanded to ia32, x64. If no arch, macro will be removed from your pattern with leading space, - and _ (so, you don’t need to worry and can reuse pattern).

@roryabraham
Copy link
Contributor

Confirmed this is fixed and desktop staging builds an un-broken.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DeployBlockerCash This issue or pull request should block deployment Engineering External Added to denote the issue can be worked on by a contributor Hourly KSv2
Projects
None yet
Development

No branches or pull requests

8 participants