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

Version comparison and build-skipping is broken #337

Open
hsanjuan opened this issue Apr 7, 2021 · 3 comments
Open

Version comparison and build-skipping is broken #337

hsanjuan opened this issue Apr 7, 2021 · 3 comments
Labels
effort/days Estimated to take multiple days, but less than a week need/analysis Needs further analysis before proceeding need/maintainers-input Needs input from the current maintainer(s) P3 Low: Not priority right now

Comments

@hsanjuan
Copy link
Contributor

hsanjuan commented Apr 7, 2021

Before: the list of releases in version could be updated and the build script would build what is needed to be built according to versions if it was not already available in dist.ipfs.io

Now: if the version file is updated to, for example, cleanup older version or remove older rc releases that should not be published anymore, the comparison between the existing version file and the current file returns a wrong list of releases to build because it lists lines that are in a position that was not expected, regardless of whether they are already built or not.

This was done in 0e510b9 and later in 19448fd . It seems there is an assumption that version files are append-only, but it is not the case. The way it was done before (or not done) just worked.

@aschmahmann @olizilla

@hsanjuan hsanjuan added the need/triage Needs initial labeling and prioritization label Apr 7, 2021
@Stebalien Stebalien added the kind/bug A bug in existing code (including security flaws) label Apr 14, 2021
@lidel lidel added effort/days Estimated to take multiple days, but less than a week need/analysis Needs further analysis before proceeding need/maintainers-input Needs input from the current maintainer(s) P3 Low: Not priority right now and removed need/triage Needs initial labeling and prioritization kind/bug A bug in existing code (including security flaws) labels May 28, 2021
@olizilla
Copy link
Member

A while back we stopped removing releases from the versions list for go-ipfs to simplify the job of seeing what has changed for automations that use it to check for new releases like https://github.com/ipfs/npm-go-ipfs/blob/master/.github/actions/check-for-go-ipfs-release/entrypoint.sh

Is removing old versions a feature that we want? I was kinda channeling the permaweb vibe and assumed we wouldn't do that.

@lidel
Copy link
Member

lidel commented May 28, 2021

cleanup older version or remove older rc releases that should not be published anymore
It seems there is an assumption that version files are append-only, but it is not the case.

@hsanjuan Why would we want to override history and remove old releases? Protecting users from known security issues?
In my experience removing packages breaks people's boxes when they do maintenance tasks involving re-installing the same version.

@hsanjuan
Copy link
Contributor Author

I only remove release candidates. They take space. It may be that something is built by mistake, I don' t know... Just wanted to highlight behavior change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort/days Estimated to take multiple days, but less than a week need/analysis Needs further analysis before proceeding need/maintainers-input Needs input from the current maintainer(s) P3 Low: Not priority right now
Projects
None yet
Development

No branches or pull requests

4 participants