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

octant: add service and use free_port in test #106816

Closed
wants to merge 1 commit into from

Conversation

alebcay
Copy link
Member

@alebcay alebcay commented Jul 29, 2022

  • Have you followed the guidelines for contributing?
  • Have you ensured that your commits follow the commit style guide?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing brew install --build-from-source <formula>)? If this is a new formula, does it pass brew audit --new <formula>?

We pass --disable-open-browser in the service since it will be running in the background and we probably don't want the user's web browser to pop open on every login/boot for this (or possibly just randomly if the application crashes and is restarted).

Use free_port to avoid potential port conflicts in the test.

Added CI-no-bottles because my understanding is that the service + test block are contained within the formula itself and should work fine with existing bottle.

@alebcay alebcay added the CI-no-bottles Merge without publishing bottles label Jul 29, 2022
@BrewTestBot BrewTestBot added go Go use is a significant feature of the PR or issue nodejs Node or npm use is a significant feature of the PR or issue labels Jul 29, 2022
@carlocab
Copy link
Member

carlocab commented Jul 29, 2022

It's not too happy with our version of npm.

We can also do this:

diff --git a/Formula/octant.rb b/Formula/octant.rb
index 48e30ff31c5..65267e81ff3 100644
--- a/Formula/octant.rb
+++ b/Formula/octant.rb
@@ -40,7 +40,7 @@ class Octant < Formula
 
     ldflags = ["-X main.version=#{version}",
                "-X main.gitCommit=#{Utils.git_head}",
-               "-X main.buildTime=#{time.iso8601}"].join(" ")
+               "-X main.buildTime=#{time.iso8601}"]
 
     tags = "embedded exclude_graphdriver_devicemapper exclude_graphdriver_btrfs containers_image_openpgp"
 

We pass --disable-open-browser in the service since it will be running in the background and we probably don't want the user's web browser to pop open on every login/boot for this (or possibly just randomly if the application crashes and is restarted).

Surprise browsers seem fun though.

Added CI-no-bottles because my understanding is that the service + test block are contained within the formula itself and should work fine with existing bottle.

👍

@alebcay
Copy link
Member Author

alebcay commented Jul 29, 2022

It's not too happy with our version of npm.

I dug into the failure a bit, the error message is emitted from npm itself (npm ci --prefer-offline --no-audit), seemingly about the version of some other thing.

Full log:

0 verbose cli /usr/local/Cellar/node/18.7.0/bin/node /usr/local/opt/node/libexec/bin/npm
1 info using [email protected]
2 info using [email protected]
3 timing npm:load:whichnode Completed in 0ms
4 timing config:load:defaults Completed in 2ms
5 timing config:load:file:/usr/local/Cellar/node/18.7.0/libexec/lib/node_modules/npm/npmrc Completed in 1ms
6 timing config:load:builtin Completed in 1ms
7 timing config:load:cli Completed in 2ms
8 timing config:load:env Completed in 1ms
9 timing config:load:file:/private/tmp/octant-20220729-62278-zm86ku/web/.npmrc Completed in 0ms
10 timing config:load:project Completed in 8ms
11 timing config:load:file:/private/tmp/octant-20220729-62278-zm86ku/.brew_home/.npmrc Completed in 1ms
12 timing config:load:user Completed in 1ms
13 timing config:load:file:/usr/local/Cellar/node/18.7.0/etc/npmrc Completed in 0ms
14 timing config:load:global Completed in 0ms
15 timing config:load:validate Completed in 0ms
16 timing config:load:credentials Completed in 1ms
17 timing config:load:setEnvs Completed in 1ms
18 timing config:load Completed in 17ms
19 timing npm:load:configload Completed in 17ms
20 timing npm:load:mkdirpcache Completed in 1ms
21 timing npm:load:mkdirplogs Completed in 1ms
22 verbose title npm ci
23 verbose argv "ci" "--prefer-offline" "--no-audit"
24 timing npm:load:setTitle Completed in 3ms
25 timing config:load:flatten Completed in 3ms
26 timing npm:load:display Completed in 6ms
27 verbose logfile logs-max:10 dir:/private/tmp/octant-20220729-62278-zm86ku/.brew_home/.npm/_logs
28 verbose logfile /private/tmp/octant-20220729-62278-zm86ku/.brew_home/.npm/_logs/2022-07-29T12_35_24_560Z-debug-0.log
29 timing npm:load:logFile Completed in 5ms
30 timing npm:load:timers Completed in 0ms
31 timing npm:load:configScope Completed in 0ms
32 timing npm:load Completed in 35ms
33 timing arborist:ctor Completed in 1ms
34 silly logfile done cleaning log files
35 timing idealTree:init Completed in 45ms
36 timing idealTree:userRequests Completed in 1ms
37 silly idealTree buildDeps
38 timing idealTree:#root Completed in 1ms
39 silly fetch manifest hosted-git-info@^4.0.2
40 http fetch GET 200 https://registry.npmjs.org/hosted-git-info 28ms (cache hit)
41 timing idealTree Completed in 90ms
42 timing command:ci Completed in 2671ms
43 verbose stack TypeError: Invalid Version: ^3.0.8
43 verbose stack     at new SemVer (/usr/local/Cellar/node/18.7.0/libexec/lib/node_modules/npm/node_modules/semver/classes/semver.js:38:13)
43 verbose stack     at compare (/usr/local/Cellar/node/18.7.0/libexec/lib/node_modules/npm/node_modules/semver/functions/compare.js:3:32)
43 verbose stack     at Object.gte (/usr/local/Cellar/node/18.7.0/libexec/lib/node_modules/npm/node_modules/semver/functions/gte.js:2:30)
43 verbose stack     at CanPlaceDep.checkCanPlaceCurrent (/usr/local/Cellar/node/18.7.0/libexec/lib/node_modules/npm/node_modules/@npmcli/arborist/lib/can-place-dep.js:173:51)
43 verbose stack     at CanPlaceDep.checkCanPlace (/usr/local/Cellar/node/18.7.0/libexec/lib/node_modules/npm/node_modules/@npmcli/arborist/lib/can-place-dep.js:157:27)
43 verbose stack     at new CanPlaceDep (/usr/local/Cellar/node/18.7.0/libexec/lib/node_modules/npm/node_modules/@npmcli/arborist/lib/can-place-dep.js:114:26)
43 verbose stack     at PlaceDep.place (/usr/local/Cellar/node/18.7.0/libexec/lib/node_modules/npm/node_modules/@npmcli/arborist/lib/place-dep.js:123:19)
43 verbose stack     at new PlaceDep (/usr/local/Cellar/node/18.7.0/libexec/lib/node_modules/npm/node_modules/@npmcli/arborist/lib/place-dep.js:73:10)
43 verbose stack     at /usr/local/Cellar/node/18.7.0/libexec/lib/node_modules/npm/node_modules/@npmcli/arborist/lib/arborist/build-ideal-tree.js:990:31
43 verbose stack     at Array.map (<anonymous>)
44 verbose cwd /private/tmp/octant-20220729-62278-zm86ku/web
45 verbose Darwin 21.5.0
46 verbose node v18.7.0
47 verbose npm  v8.15.0
48 error Invalid Version: ^3.0.8
49 verbose exit 1
50 timing npm Completed in 2778ms
51 verbose unfinished npm timer idealTree:buildDeps 1659098127289
52 verbose unfinished npm timer idealTree:node_modules/app-builder-lib 1659098127293
53 verbose code 1
54 error A complete log of this run can be found in:
54 error     /private/tmp/octant-20220729-62278-zm86ku/.brew_home/.npm/_logs/2022-07-29T12_35_24_560Z-debug-0.log

@cho-m
Copy link
Member

cho-m commented Jul 29, 2022

^3.0.8 would probably mean the hosted-git-info resolution https://github.com/vmware-tanzu/octant/blob/v0.25.1/web/package.json#L155

Looks like resolutions are a non-native-to-npm feature supported by using npm-force-resolutions. Some other error reports but they range across multiple node/npm versions rogeriochaves/npm-force-resolutions#56.

@alebcay
Copy link
Member Author

alebcay commented Jul 31, 2022

I've tried both of the workarounds originally mentioned in the linked issue there but unfortunately those don't seem to fix it (the particular version string that it's failing on does change, though).

@github-actions
Copy link
Contributor

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@github-actions github-actions bot added the stale No recent activity label Aug 21, 2022
@github-actions github-actions bot closed this Aug 30, 2022
@github-actions github-actions bot added the outdated PR was locked due to age label Sep 30, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CI-no-bottles Merge without publishing bottles go Go use is a significant feature of the PR or issue nodejs Node or npm use is a significant feature of the PR or issue outdated PR was locked due to age stale No recent activity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants