Skip update checks for bundled packages

[confused-Techie]

This PR allows PPM to skip checking for updates for bundled packages within Pulsar.

This only works if the package is not a valid git repository, which is true for the vast majority of our bundled packages.

Although, some outliers to this exist, those can be determined by checking the remaining items here. As for all other of the 81 bundled packages, this should allow them to be silently skipped. Assumed to always be up to date by PPM.

This is done by checking the repository of the package.json of the package and determining if it's the same as the repository of Pulsar itself.

[DeeDeeG]

As a reviewer for this repo:

I'm sort of surprised none of the tests complain about this, although I guess exercising this specific scenario is outside the scope of any tests at the moment.

Ideally we might want to add a test for this, maybe as a follow-up perhaps, but I'm not feeling like asking for that for an approve on this, personally. If you want to, that'd be cool.

As a reviewer for pulsar-edit org in general:

Ideally we revert this as soon as we find a way to not ask ppm for these updates for core packages?? Since that seems like a cleaner location for the change to go, over in the requesting-the-check side, not the implementing-the-check side.

EDIT: Maybe this PR you just opened does what I was hoping we'd do instead?: https://github.com/pulsar-edit/pulsar/pull/711/files

For now, I think this is worth doing, and this is a small change scoped specifically to our in-house repos. (We don't generate that repository URL in new package templates, right??)

Alrighty, so with that said, approved. 👍

[DeeDeeG]

If you wanna address some of the feedback, then I can wait, otherwise, I am inclined to merge.

(I'm feeling a little move-fast about this PR today myself, but any efforts to arrive at even better approach is appreciated and I'd wait however long it'd take for it if you were so inclined to find improvements along those lines as I mentioned. But they are optional, since this is kind of a hack and workaround anyway. The only alternative approaches we might find could still end up being various degrees of hacks/workarounds.)

And once I review whatever all else has been committed since ppm was synced to core repo, I (or someone else if I can't today) can do a ppm bump in core repo.

[confused-Techie]

@DeeDeeG So on what you've mentioned here.

My other PR over in pulsar doesn't address what you mentioned.

The difficult part about just not asking ppm to check for updates of certain packages, is that settings-view actually doesn't ask for updates to any specific packages.

Instead settings-view just runs the generic command to ask PPM to check for updates for all installed packages. Which would include our bundled ones. So really this may be the only way to stop this behavior from occurring.

As for adding a test, I'd be more than happy to take a look to see if we can do that. But I'd like to say, I don't see any way we can easily revert this PR later down the road without significant changes.

[DeeDeeG]

The difficult part about just not asking ppm to check for updates of certain packages, is that settings-view actually doesn't ask for updates to any specific packages.

@confused-Techie so there's no obvious way to check the repository of "all packages" in this instance and not ask for the update?

Is it really asking ppm to "go check all the packages at once to see if upgrades are needed?" Oh, wait... Yes it is.

% ppm upgrade --help

Usage: ppm upgrade
       ppm upgrade --list
       ppm upgrade [<package_name>...]

Upgrade out of date packages installed to ~/.pulsar/packages

This command lists the out of date packages and then prompts to install
available updates.

[ . . . snipped . . . ]

Okay, I see why it wouldn't be easy to do this part over at core repo, and why we have to fix it in two different places, since we're fetching updates for packages for two separate purposes, in two separate ways across settings-view package.

Thanks for the clarification nudging me to check closer what command this PR is editing, I understand better now.

[DeeDeeG]

CI is green with the new test, which is a little above and beyond for this kind of quick fix, even if I did request it. Thank you.

And thank you for the context replying to my previous review comments.

Tests pass (including new test), LGTM! Re-approving.

EDIT to add: And let's merge! 🚀

[confused-Techie]

@DeeDeeG Exactly, which is unfortunate. But yeah settings-view doesn't even know what packages are being checked. So it'd be a significant code change to even make it aware of the packages to them go ahead and not check them. But hopefully this fix isn't too hacky to remain in the codebase for a bit of time.

If we really worried about it's implementation, we could add a flag to the command, such as --skip-core which would optionally enable this feature, so that it worked here, but otherwise wouldn't provide any strange behavior for end users

[confused-Techie]

Lets merge, woot!

[DeeDeeG]

On closer look, this ppm upgrade command is already checking only packages under ~/.pulsar/packages. So, bundled packages should be skipped right out of the gate, so to speak. (And it seems to be already not hitting the backend API for packages installed via ppm link or installed from git URLs either, at least in testing on my machine.)

So, even without this PR, ppm upgrade should already be ignoring any truly bundled-with-the-editor packages already, and most (all?) ways folks would get a package with repository matching our core repo URL.

Furthermore, with familiarity of how the packages' package.jsons are formatted on-disk in Pulsar, this seems to be another case where packages get the object form of repository, where repository.url is the real URL we are looking for, and it ends in .git.

For example, when I run ./bin/ppm upgrade --verbose, it is only trying to check for upgrades to my two non-bundled, non-ppm linked, installed packages from the Pulsar package registry.

With some console.log() peppered into the code:

% ./bin/ppm outdated           
NOTTTT skippinggggg
pack.repository is: 
{ type: 'git', url: 'git+https://github.com/dracula/atom.git' }
NOTTTT skippinggggg
pack.repository is: 
{ type: 'git', url: 'git+https://github.com/dracula/atom-ui.git' }
Package Updates Available (0)
└── (empty)

Verbose output (click to expand):

% ./bin/ppm outdated --verbose
NOTTTT skippinggggg
pack.repository is: 
{ type: 'git', url: 'git+https://github.com/dracula/atom.git' }
NOTTTT skippinggggg
pack.repository is: 
{ type: 'git', url: 'git+https://github.com/dracula/atom-ui.git' }
REQUEST {
  url: 'https://api.pulsar-edit.dev/api/packages/dracula-syntax',
  json: true,
  strictSSL: true,
  headers: { 'User-Agent': 'npm/6.14.19-pulsar1-1 node/v16.0.0 darwin x64' },
  callback: [Function (anonymous)],
  method: 'GET'
}
REQUEST {
  url: 'https://api.pulsar-edit.dev/api/packages/dracula-ui',
  json: true,
  strictSSL: true,
  headers: { 'User-Agent': 'npm/6.14.19-pulsar1-1 node/v16.0.0 darwin x64' },
  callback: [Function (anonymous)],
  method: 'GET'
}
REQUEST make request https://api.pulsar-edit.dev/api/packages/dracula-syntax
REQUEST make request https://api.pulsar-edit.dev/api/packages/dracula-ui
REQUEST onRequestResponse https://api.pulsar-edit.dev/api/packages/dracula-syntax 200 {
  'content-type': 'application/json; charset=utf-8',
  vary: 'Accept-Encoding',
  'x-powered-by': 'Express',
  'x-ratelimit-limit': '600',
  'x-ratelimit-remaining': '599',
  'x-ratelimit-reset': '1694789635',
  'ratelimit-limit': '600',
  'ratelimit-remaining': '599',
  'ratelimit-reset': '95',
  etag: 'W/"1a0a-Q4SF5B2tJH9bgIKaZSXrQTJWgJg"',
  'x-cloud-trace-context': 'd488d9ef7af730ba45be48e93e2c6f03',
  date: 'Fri, 15 Sep 2023 14:52:19 GMT',
  server: 'Google Frontend',
  'content-length': '6666',
  connection: 'close'
}
REQUEST reading response's body
REQUEST finish init function https://api.pulsar-edit.dev/api/packages/dracula-syntax
REQUEST response end https://api.pulsar-edit.dev/api/packages/dracula-syntax 200 {
  'content-type': 'application/json; charset=utf-8',
  vary: 'Accept-Encoding',
  'x-powered-by': 'Express',
  'x-ratelimit-limit': '600',
  'x-ratelimit-remaining': '599',
  'x-ratelimit-reset': '1694789635',
  'ratelimit-limit': '600',
  'ratelimit-remaining': '599',
  'ratelimit-reset': '95',
  etag: 'W/"1a0a-Q4SF5B2tJH9bgIKaZSXrQTJWgJg"',
  'x-cloud-trace-context': 'd488d9ef7af730ba45be48e93e2c6f03',
  date: 'Fri, 15 Sep 2023 14:52:19 GMT',
  server: 'Google Frontend',
  'content-length': '6666',
  connection: 'close'
}
REQUEST end event https://api.pulsar-edit.dev/api/packages/dracula-syntax
REQUEST has body https://api.pulsar-edit.dev/api/packages/dracula-syntax 6666
REQUEST emitting complete https://api.pulsar-edit.dev/api/packages/dracula-syntax
REQUEST onRequestResponse https://api.pulsar-edit.dev/api/packages/dracula-ui 200 {
  'content-type': 'application/json; charset=utf-8',
  vary: 'Accept-Encoding',
  'x-powered-by': 'Express',
  'x-ratelimit-limit': '600',
  'x-ratelimit-remaining': '598',
  'x-ratelimit-reset': '1694789635',
  'ratelimit-limit': '600',
  'ratelimit-remaining': '598',
  'ratelimit-reset': '95',
  etag: 'W/"672-IQ59bu7SWhblVpHuXNh89mXJzyI"',
  'x-cloud-trace-context': '1e0d859733f76f54e9d0c3e124bd73df',
  date: 'Fri, 15 Sep 2023 14:52:19 GMT',
  server: 'Google Frontend',
  'content-length': '1650',
  connection: 'close'
}
REQUEST reading response's body
REQUEST finish init function https://api.pulsar-edit.dev/api/packages/dracula-ui
REQUEST response end https://api.pulsar-edit.dev/api/packages/dracula-ui 200 {
  'content-type': 'application/json; charset=utf-8',
  vary: 'Accept-Encoding',
  'x-powered-by': 'Express',
  'x-ratelimit-limit': '600',
  'x-ratelimit-remaining': '598',
  'x-ratelimit-reset': '1694789635',
  'ratelimit-limit': '600',
  'ratelimit-remaining': '598',
  'ratelimit-reset': '95',
  etag: 'W/"672-IQ59bu7SWhblVpHuXNh89mXJzyI"',
  'x-cloud-trace-context': '1e0d859733f76f54e9d0c3e124bd73df',
  date: 'Fri, 15 Sep 2023 14:52:19 GMT',
  server: 'Google Frontend',
  'content-length': '1650',
  connection: 'close'
}
REQUEST end event https://api.pulsar-edit.dev/api/packages/dracula-ui
REQUEST has body https://api.pulsar-edit.dev/api/packages/dracula-ui 1650
REQUEST emitting complete https://api.pulsar-edit.dev/api/packages/dracula-ui
Package Updates Available (0)
└── (empty)

---

(And it does have some support for checking for updates for packages from git URLs, but these are handled with git commands and fetching from the packages' respective repos, not hitting the package backend API server.)

[DeeDeeG]

I dunno if we want to revert the change, considering this is such a small and not-hurting-anything change, but it also isn't doing the thing it's meant to be doing in real-world testing, I think. (Given some further research into things since this was merged.)

Sorry to be a bother about it, I just noticed and wanted to share what I found.

[confused-Techie]

@DeeDeeG Great catch here, and now that I think about it, you are totally right. This wouldn't ever see any packages that are bundled.

So yeah this is kinda pointless, and while harmless, I'd hate for it to add uncertainty into how PPM behaves to someone reading it in the future. So seems best to revert this PR.

[DeeDeeG]

There's a shiny "Revert" button next to the "merge" event if scrolling up the history of this PR.

I'm somewhat curious to try it. Hmmmmmmmmmmmm. I might try that in a bit if no objections, or if you want to try it, go for it.

EDIT: Ah. When hovered, it says it will "Create a new pull request to revert these changes". So, nothing too crazy. Makes sense.

[confused-Techie]

@DeeDeeG I've never seen this option. So that sounds like a plan then, lets give it a wirl