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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a new entry for our custom version bump #8196

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

supechicken
Copy link
Member

@supechicken supechicken commented Apr 18, 2023

Not tested yet

Currently, we use @_ver to store the upstream version and put our custom subversion inside of version "#{@_ver}-...", but having two different variables for version might confuse new contributors.

This PR adds a new entry called revision for our custom subversion and is optional for backward compatibility, here are some examples:

# before
@_ver = '1.1'
version "#{@_ver}-1"

# after
version '1.1'
revision 1

# before
@_ver = '1.0'
version @_ver

# after
version '1.0'
revision null # can be omitted

####
# Use `@version` to get the upstream version
# Use `@revision` to get the subversion

I only applied it to some packages as there are so many packages that use @_ver for versioning 馃槄

Run the following to get this pull request's changes locally for testing.

CREW_TESTING_REPO=https://github.com/supechicken/chromebrew.git CREW_TESTING_BRANCH=add_release CREW_TESTING=1 crew update

@supechicken supechicken marked this pull request as draft April 18, 2023 18:38
lib/package.rb Outdated Show resolved Hide resolved
Copy link
Collaborator

@uberhacker uberhacker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this idea! Bump crew version? Obviously, we'll need to update all the affected packages before this can be merged. Another thing to consider is all the packages that use @_ver throughout the package need to use that version.

@satmandu
Copy link
Member

So we're also doing some tagging of python3 packages with the python3 version, and i think we're headed towards doing the same for perl.

But there are other version sub-tags which might make sense too. For instance, when building with LTO, packages complain when a library is built with a different version of GCC.

Might it make sense to also be able to tag packages with the GCC version used to build it? Obviously a revision tag would make sense for the rebuild, but it would also be nice to independently be able to scan packages to see what version of GCC was used to build it.

@satmandu
Copy link
Member

There's an ur-question here of how much package metadata should go in the package file, as opposed to being put elsewhere.

packages/apng2gif.rb Outdated Show resolved Hide resolved
packages/abseil_cpp.rb Outdated Show resolved Hide resolved
@supechicken
Copy link
Member Author

Current progress on replacing @_ver in all packages: 119/549

It might take me a week to complete it... 馃拃

@supechicken
Copy link
Member Author

So we're also doing some tagging of python3 packages with the python3 version, and i think we're headed towards doing the same for perl.

But there are other version sub-tags which might make sense too. For instance, when building with LTO, packages complain when a library is built with a different version of GCC.

Might it make sense to also be able to tag packages with the GCC version used to build it? Obviously a revision tag would make sense for the rebuild, but it would also be nice to independently be able to scan packages to see what version of GCC was used to build it.

How about creating a new suffix tag for them?

@supechicken
Copy link
Member Author

supechicken commented Jun 12, 2023

Thanks to @Zopolis4, now we have 362/549 (187 completed) left :)

@Zopolis4
Copy link
Collaborator

Speaking of-- it looks like you're manually removing @_ver from packages. What I've been doing is writing and using a series of sed scripts, which requires much less busywork. In the interests of saving time and making sure we don't double up on work, would it make more sense for me to just continue on as I am using my scripts? I plan to submit PRs for every @_ver usage that doesn't need this PR.

After that, I'll submit PRs that remove the usages of @_ver that do need this PR, and we can merge those after this is merged.

Is that ok? I don't want to step on your toes, but I also don't want you to have to go through ~400 packages manually when I can automate the process.

@supechicken
Copy link
Member Author

The reason that I don't use sed is to prevent any possible breakage. (e.g. there are some packages that involve @_ver in another variable like @_ver_prelastdot. It won't work by just simply replacing @_ver with version as those variables were defined before version is defined and thus throwing errors)

@Zopolis4
Copy link
Collaborator

I've got some fallbacks for that, and I do test the packages.

@supechicken supechicken marked this pull request as ready for review July 23, 2023 14:32
@version = CREW_KERNEL_VERSION == '4.14' ? "#{CREW_KERNEL_VERSION}-1" : CREW_KERNEL_VERSION
version @version
version = CREW_KERNEL_VERSION == '4.14' ? "#{CREW_KERNEL_VERSION}-1" : CREW_KERNEL_VERSION
version version
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  revision 1
  version CREW_KERNEL_VERSION == '4.14' ? "#{CREW_KERNEL_VERSION}-#{revision}" : CREW_KERNEL_VERSION

?

@@ -9,7 +9,7 @@ class Binutils < Package
version "#{@_ver}-1"
license 'GPL-3+'
compatibility 'all'
source_url "https://ftpmirror.gnu.org/binutils/binutils-#{@_ver}.tar.bz2"
source_url "https://ftpmirror.gnu.org/binutils/binutils-#{version}.tar.bz2"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like you forgot to edit the rest of the package here?

Comment on lines -248 to +264
"#{name}.#{Time.now.utc.strftime('%Y%m%d%H%M%S')}.dir"
return "#{name}.#{Time.now.utc.strftime('%Y%m%d%H%M%S')}.dir"
end

def self.is_binary?(architecture)
if !@build_from_source && @binary_url && @binary_url.key?(architecture)
return true
else
return false
end
return (!@build_from_source && @binary_url && @binary_url.key?(architecture))
end

def self.is_source?(architecture)
if is_binary?(architecture) || is_fake?
return false
else
return true
end
return !(is_binary?(architecture) || is_fake?)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If these are just general improvements to the code,
does it make more sense to seperate these into another PR?

Comment on lines -6 to +11
version '24eaef0d11e590643e65d188b017b49414d81cc2'
@commit = '24eaef0d11e590643e65d188b017b49414d81cc2'
version @commit[0...7]
license 'MIT'
compatibility 'all'
source_url 'https://github.com/internetwache/GitTools.git'
git_hashtag version
git_hashtag @commit
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm conscious of the fact that this is still quite similar to using @_ver-- is there any other way to do this which doesn't introduce an additional variable?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants