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

Fix ruby Portfile #37

Merged
merged 1 commit into from Jul 1, 2019
Merged

Fix ruby Portfile #37

merged 1 commit into from Jul 1, 2019

Conversation

Korusuke
Copy link
Member

Fixes #36

@reneeotten
Copy link
Contributor

thanks @Korusuke the changes in the template look good and work as expected!

I just had another look at the error message [ERROR ] [Backend] Could not determine the type of the source archive that you now successfully suppress, but on second thought there shouldn't be a need for that.

@Steap added Archive to upt-rubygems and the frontend indeed correctly returns an archive. I think that there is a bug in the get_archive function in upt itself. If you add some print statements and run upt package -f rubygems -b macports treetop then you'll see the following:

self.archives = [<upt.upt.Archive object at 0x106664c50>]
archive.archive_type = ArchiveType.RUBYGEM

which seems all correct. However, archive_type in this function is still archive_type = ArchiveType.SOURCE_TARGZ and that means that it's filtered out in lines 187-188.

@Korusuke
Copy link
Member Author

Yes, agree even I think that it should be fixed in upt as get_archive() sets the archive type as targz2 by default.
As of now I have changed the code so it makes a exception while calling the function and not in error as that is more appropriate this way but I still think that this should be handled by upt.

@reneeotten
Copy link
Contributor

I guess my choice words using "bug" is probably too strong / wrong, it might actually be a feature ;) In all cases except for upt-rubygems the SOURCE_TARGZ is the correct type, so it makes sense that this is the default. So perhaps your change to only call the _get_archive function with specifying the RUBYGEM archive type is the correct thing to do, I am not sure whether upt can take care of this automagically - lets see what @Steap has to say.

else:
archive_name = self.upt_pkg.get_archive(
self.upt_pkgArchiveType.RUBYGEM).filename

Copy link
Collaborator

Choose a reason for hiding this comment

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

What you could do here is define an "archive_type" class attribute for all subclasses of MacPortsPackage (just like the "template" attribute) that would be equal to "upt.ArchiveType.RUBYGEM" or "upt.ArchiveType.SOURCE_TARGZ" (note that the latter does not necessarily refer to a tar.gz, because I fucked up when naming it :p).

You would then write:

archive_name = self.upt_pkg.get_archive(self.archive_type).filename

@Steap
Copy link
Collaborator

Steap commented Jun 29, 2019

lets see what @Steap has to say.

Yep, you're right. get_archive() defaults to looking for a source archive, because that is the most common use case, but one may want to look for a Python wheel, a Ruby gem, etc.

@reneeotten
Copy link
Contributor

thanks @Korusuke I am happy with this, it works exactly as intended now! @Steap if you agree, please do merge this PR or approve the changes and I'll get to it tomorrow.

@Steap
Copy link
Collaborator

Steap commented Jul 1, 2019

@reneeotten It seems OK, but I'll let you merge, you understand these Ruby/Macports issues better than me :)

@reneeotten reneeotten merged commit dac1340 into macports:master Jul 1, 2019
@reneeotten reneeotten mentioned this pull request Jul 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Ruby ports
3 participants