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

Core and bundled packages link to the wrong package via the Pulsar package repository #353

Closed
5 tasks done
Daeraxa opened this issue Jan 28, 2023 · 13 comments
Closed
5 tasks done
Labels
bug Something isn't working

Comments

@Daeraxa
Copy link
Member

Daeraxa commented Jan 28, 2023

Thanks in advance for your bug report!

  • Have you reproduced issue in safe mode?
  • Have you used the debugging guide to try to resolve the issue?
  • Have you checked our FAQs to make sure your question isn't answered there?
  • Have you checked to make sure your issue does not already exist?
  • Have you checked you are on the latest release of Pulsar?

What happened?

The package cards in Settings > Packages allow you to click the title which will take you to the page on web.pulsar-edit.dev for that package.

This is fine for community packages but we have a problem when it comes to core and bundled packages

If you select a bundled core package such as about and click the link, it will take you to https://web.pulsar-edit.dev/packages/about which is the packge from Atom linked to the original atom/about repo - https://github.com/atom/about.

Non-bundled core packages such as github will do the same, this should take you to our own package page for it but we don't actually seem to have our own core packages published to the backend like Atom did with theirs.

This means that our updated packages are still linking via the name to the original Atom ones. (Take my example here with a pinch of salt as the github package I think is still being used as an older dependency from before the update to the pulsar-edit org in the package.json). i.e because the package in Pulsar is called "github" then this is what gets linked to on the backend because the names are unique and that package is Atom's.

The main issue here is simply that the "owner" of the core package names seems to be Atom in the backend. I'm guessing we either need to remove the "duplicate" Atom packages from the backend or we need to change their name to indicate that these are legacy Atom packages and not Pulsar ones.
We would then need to push Pulsar packages to the backend - the question is 1) how and 2) if we would want to even do this with bundled packages - would we instead need to link directly to the relevant packages dir in the Pulsar repo as part of the point is that we no longer need to publish them as discrete updates.

Pulsar version

1.101.0-dev

Which OS does this happen on?

🪟 Windows

OS details

10

Which CPU architecture are you running this on?

64-bit(x86_64)

What steps are needed to reproduce this?

Open the package install page and click the title of any Pulsar core package on the package card.

Additional Information:

No response

@Daeraxa Daeraxa added the bug Something isn't working label Jan 28, 2023
@DeeDeeG
Copy link
Member

DeeDeeG commented Feb 1, 2023

These links are actually handled locally by the settings-view package, I think, by reading each package's package.json as a source of truth for where to link to. (Or to the extent the backend stores this info, it was also at some point ultimately reading from whatever package.json was uploaded with the package at the time it was published to the backend.)

This is two issues, I think.

  • One issue is for bundled/core packages.
    • We just need to update the repository field of each respective package's package.json, and get that changed version of the package updated/bundled with core.
    • This solves the issue for the bundled copy of the package, in the "installed packages" view.
  • The other issue is the copies of the packages in the package registry.
    • These package versions were migrated over from the Atom package registry as-is, and haven't been updated since.
    • These are generally stale, as we haven't been publishing core packages to our new package registry.
    • We can either publish newer versions of these packages with the updated repository field to our package registry, or...
    • Alternatively we could consider de-listing these core/bundled packages from our package registry. Since it's kind of redundant to our direct use of GitHub repo URLs, and I don't think it's recommendable to grab the old stale versions from the package registry...?
    • Addressing this improves the accuracy/recency of data/links stored about the packages on the backend and displayed on the packages website.

@Daeraxa
Copy link
Member Author

Daeraxa commented Feb 1, 2023

So I left the non-bundled core package out of this because most have actually had updates to the package.json already and just need a version bump in the Pulsar dependencies. The issue itself manifests itself regardless.

The bundled core packages already have been updated with the new json, for example about:

  "description": "View useful information about your Pulsar installation.",
  "repository": "https://github.com/pulsar-edit/pulsar",

My assumption is that the issue is partially backend related because the URL is just for the "unique" package name and not owner + package name so it links to the existing Atom packages as we don't have our own.

There was a discussion on Discord about it here - https://discord.com/channels/992103415163396136/992110095641088001/1068922562370875544 where we discussed possible approaches to it.

@DeeDeeG
Copy link
Member

DeeDeeG commented Feb 1, 2023

Okay, well that is definitely due to outdated content hosted at the package registry for those packages.

There was some way to transfer ownership of a package, I think, but we could force upload the updated package version some way, I bet.

Once newer versions of the packages would be uploaded to the package registry, the links would reflect the corrections we've already made (but which have never been pushed to the package registry yet, since we just don't publish them there like Atom team used to do.)

And I think the other sensible approach would have to be de-listing / deleting them.

Fudging it by updating the info in the DB to not match the true metadata from those old versions seems somehow wrong to me, but that is the other option @confused-Techie mentioned.

@confused-Techie
Copy link
Member

Yeah I'll add to this here, it does seem somehow wrong to modify the data from the Atom team, while at the same time if we delete the listings then add our own it's not that much different effectively, so I'm less reserved than I originally was at just changing the data. Although it's important to note that because we no longer are uploading them properly, some packages can't just be updated to point to our repository link, because of build steps associated with the act of publishing them.

So there's some considerations, if we just want them to show our name then we can do that (Since I am able to update any aspect of a listed package with some ease) but this may be something we wanna vote on or properly research to find any possibilities of flaws

@DeeDeeG
Copy link
Member

DeeDeeG commented Feb 2, 2023

My ideal case, for the github package as an example is:

  • the old github package versions are the Atom ones
  • We bump the version and update the repository and other metadata to say Pulsar instead of Atom, since it's our forked version it should be re-branded anyway
  • Our version is simply the "latest" version, old versions remain available

If we have to cheat/fudge to gitve pulsar-edit org members ownership of the github package on our backend server (without any sort of prefixed name), I would be okay with that.

Is this hard to do?

@DeeDeeG
Copy link
Member

DeeDeeG commented Feb 2, 2023

I just remembered having Pulsar-branded metadata for, say the github package on the backend server kind of implies it's a valid place to get the package... but we're not really planning to upload it to there... IDK.

Maybe we should just add a little disclaimer to the metadata/README of github package on the backend server. Like:

"This is the old Atom version of the github package. It is left for historical, archival purposes. Please see https://github.com/pulsar-edit/github for the latest version of this package from the pulsar-edit org."

Or something like that?

@confused-Techie
Copy link
Member

So I want to say there is currently some discussion about where the repository link will actually go. So some of this may have to wait, and it may actually be a vote towards including the repository in the versions themselves rather than the full package.

But no matter the answer, essentially the first new version publish would have to be manually crafted on the database, which isn't easy but is perfectly doable. Once it's done once we could then essentially take over the package to let anyone from the pulsar org (anyone with write access that is) then push updates.

But really I have to wonder, since this is a core package does it need to be included in the backend at all?

@mjrodgers
Copy link

But really I have to wonder, since this is a core package does it need to be included in the backend at all?

In theory even though it's a core package, I could uninstall it through Preferences->Packages. But then if I wanted to reinstall it using Preferences->Install I would only be able to install the old Atom one, since that's the one published.

So my vote would be to publish the new versions of core packages to the repository.

@Daeraxa
Copy link
Member Author

Daeraxa commented Jun 8, 2023

You shouldn't be able to uninstall a core package - only disable it.

@confused-Techie
Copy link
Member

Exactly like you've mentioned, it may make sense to totally remove it. But we absolutely could start publishing it again to the backend if only to not break the current system of linking to extra data about the package.

I've learned more about how we can easily 'takeover' previously published packages and honestly it's a pretty simple process. We just need to make one manual change on the backend to allow us to takeover publishing of any given package, then we publish a new version of that package via APM.

But this package would not be installable. Since installable code of packages is found via tarball links on tags for each package, so since they are bundled those no longer exist, which means either we need to allow the backend to ignore collecting tarball info, or know that quietly every core package actually contains a tarball link to releases of pulsar as a whole

@savetheclocktower
Copy link
Contributor

I'm so very confused about this. After reading the problem, my thoughts were these:

  1. Links in settings-view for core packages should not link to the package repository. We have ways of detecting built-in packages, so we should be able to have those links do nothing at all — or, failing that, to link to the GitHub repo. I can't think of a reason why a Pulsar user would need to jump from a package's listing in settings-view to that package's page in the PPR.
  2. I can imagine someone looking up the package on the PPR, so the PPR page should still exist for posterity, and to squat the name. But I like @DeeDeeG's suggestion of explaining that this is a built-in package; furthermore, the install button should be absent or disabled.

I understand why we've gotten off onto tangents, but the least disruptive thing for me would be to just add notices to those PPR pages instead of trying to do something clever with delisting or changing the author.

@confused-Techie
Copy link
Member

@savetheclocktower You do make a very good point here.

I suppose at the very least I'd like to see us update the readme for these built in package's, but beyond that I suppose it does make total sense to display warnings or other notices that this is a built in package, as well as like you said excluding the install button for them. That is probably like you mentioned the simplest solution here.

But I appreciate you coming in and getting things a bit more on track, and I'll get started with the changes needed for the frontend to support this behavior, and we can discuss further on the language we would want to use here

@confused-Techie
Copy link
Member

Forgot to mention a bit ago, that as of now if you click the link to a bundled package, the frontend website will warn that this package is bundled within Pulsar, and will remove any install buttons from it.

Some packages have also been updated to remove the Atom name from the repo, although not all have.

At the very least I feel like we can say this one has been resolved for the fixing the issue of confusion.

Reference

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants