macports / upt-macports

The Universal Packaging Tool (upt) backend for MacPorts
https://framagit.org/upt/upt
BSD 3-Clause "New" or "Revised" License
9 stars 12 forks source link

Add package overwrite option #58

Open jrjsmrtn opened 4 years ago

codecov[bot] commented 4 years ago

Codecov Report

Merging #58 into master will increase coverage by 0.05%. The diff coverage is 71.42%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #58      +/-   ##
==========================================
+ Coverage   90.64%   90.69%   +0.05%     
==========================================
  Files           1        1              
  Lines         171      172       +1     
  Branches       10       10              
==========================================
+ Hits          155      156       +1     
  Misses         16       16
Impacted Files Coverage Δ
upt_macports/upt_macports.py 90.69% <71.42%> (+0.05%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 216634f...6b40111. Read the comment docs.

jrjsmrtn commented 4 years ago

The https://framagit.org/upt/upt/merge_requests/4 pull request adds an --overwrite option to the upt command and a corresponding overwrite keyword argument to the package() function and backend.create_package() method signatures.

This allow to do a upt --recursive --overwrite and automatically overwrite package and dependencies in one operation.

This pull request adds overwrite support to the upt-macports backend.

reneeotten commented 4 years ago

@jrjsmrtn this PR is less likely to fly... If there is already a Portfile present, the idea would be to update that one instead of simply overwriting it. We assume that maintainer has "perfected" the Portfile already and we do not necessarily want to replace that with an automatically generated file.

As you can imagine such "update" feature is a bit more complex and you can read some discussion about this in the relevant Issue/PR. There is a prototype by @Steap for the update feature here: https://github.com/Steap/upt-macports-update and -to be honest- is waiting for feedback and testing from me... Please feel free to give that a try!

jrjsmrtn commented 4 years ago

Yes, I saw the effort on update but I think update and overwrite are answers to two different use cases, and are complementary.

I’m trying to package a Ruby-based software for MacPorts. That software has more than 130 direct Gem dependencies. Performing a « upt package —recursive » on the first dependency, bootsnap, generates 32 Portfiles before it breaks.

So, at this time, I’m learning and fixing upt, upt-rubygems and upt-macports, one issue at a time.

Non-backslashed double-quotes in Portfile description fields are fixed in upt-macports. Summary/description fields are fixed in upt, upt-rubygems and upt-macports. Next in line: some Ruby licenses are not recognized and not explicitly reported, my recursive build on bootsnap breaks with an InvalidSpecifier error and I had to add lots of debug messages to even know which dependency was the issue,… :-p

So, in this phase, I don’t need an « upt update ». I need to restart a recursive upt package to regenerate all dependencies’ Portfiles, until I fix all UPT-related issues.

Once all Portfiles are generated, I’ll start to build and install dependencies’ ports, manually tweak them if needed and submit them to MacPorts.

It’s only after that phase that I would (gladly) use a « upt update ».

Does it make sense to you? :-)

G.

Le 6 déc. 2019 à 03:39, Renee Otten notifications@github.com a écrit :

@jrjsmrtn https://github.com/jrjsmrtn this PR is less likely to fly... If there is already a Portfile present, the idea would be to update that one instead of simply overwriting it. We assume that maintainer has "perfected" the Portfile already and we do not necessarily want to replace that with an automatically generated file.

As you can imagine such "update" feature is a bit more complex and you can read some discussion about this in the relevant Issue/PR. There is a prototype by @Steap https://github.com/Steap for the update feature here: https://github.com/Steap/upt-macports-update https://github.com/Steap/upt-macports-update and -to be honest- is waiting for feedback and testing from me... Please feel free to give that a try!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/macports/upt-macports/pull/58?email_source=notifications&email_token=AABFIPF6C4Q55FIVMOTQXXLQXG3NNA5CNFSM4JVYVBPKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGC2STA#issuecomment-562407756, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABFIPDSEIBTRHJ5JYFRRIDQXG3NNANCNFSM4JVYVBPA.

reneeotten commented 4 years ago

@jrjsmrtn I see your point and you can certainly make such changes locally, but I am not sure if it will be added to upt (which isn't my call anyway).

Having said that, as a possible workaround without changing anything in upt: why don't you do the packaging in a directory that you just remove before the next round?

Steap commented 4 years ago

Summary/description fields are fixed in upt, upt-rubygems and upt-macports.

Yes, this was in upt-rubygems 0.4 and upt 0.11.

Next in line: some Ruby licenses are not recognized and not explicitly reported

Can you tell me what packages have issues with licenses? My understanding is that people can write whatever they want in the "license" field, making it hard to properly guess what license is used.

my recursive build on bootsnap breaks with an InvalidSpecifier error

I fixed this in upt-rubygems 0.4.1, tell me whether it works for you :)

Regarding the "overwrite" feature, it is something I had in mind, and I'm not against adding it at some point, but two things should be kept in mind:

1) It is going to be really easy to shoot yourself in the foot when using it with "--recursive"

2) As Renee suggested, "rm -rf" (or using version control) could probably help with your use case. What do you think?

Steap commented 4 years ago

Regarding the issues with licenses:

1) I pushed a commit that adds a few licenses to spdx2macports.json (I meant to create a pull request through the "edit online" feature of Github, but it was automagically pushed since I got write rights on this repo, oops) https://github.com/macports/upt-macports/commit/e187980cd6c7d0b4ff732aa99e6e7fbd9bdf47ab

2) I tried packaging bootsnap and proposed a few patches upstream to improve license detection (see https://gist.github.com/Steap/ddff705d44283ea5fa39bc73b517615a)

jrjsmrtn commented 4 years ago

Can you tell me what packages have issues with licenses? My understanding is that people can write whatever they want in the "license" field, making it hard to properly guess what license is used.

I was blocked by some « Apache 2.0 » licenses without a dash and a « Ruby! » with an exclamation point.

Instead of adding more license names to the RubyGemsFrontend._guess_licenses().ruby_to_upt mapping, I added a simple but extensible translation table:

license_transtable = str.maketrans({ " ": "-", "!": None, })

…and used it to translate license names and reduce the number of items in the mapping.

I also added explicit logging of unknown license names in RubyGemsFrontend._guess_licenses().

I’ll submit a PR in the coming days.

my recursive build on bootsnap breaks with an InvalidSpecifier error

I fixed this in upt-rubygems 0.4.1, tell me whether it works for you :)

The following specifiers or versions were breaking:

I created a RubyGemsSpecifier class, inheriting from packaging.specifiers.LegacySpecifier and using regexes from rubygems/specifier.rb and rubygems/version.rb.

All patterns are now parsed except for the four-digits version: it's not supported by RubyGemsFrontend._fix_twiddle_waka_expr() nor the semver module. I’m working on it right now and intend to submit a PR later this week.

G.

Steap commented 4 years ago

All patterns are now parsed except for the four-digits version: it's not supported by RubyGemsFrontend._fix_twiddle_waka_expr() nor the semver module. I’m working on it right now and intend to submit a PR later this week.

I don't think the twiddle-wakka is supposed to work with anything else than Semver versions, to be honest :-/

reneeotten commented 4 years ago
1. I pushed a commit that adds a few licenses to spdx2macports.json (I meant to create a pull request through the "edit online" feature of Github, but it was automagically pushed since I got write rights on this repo, oops) [e187980](https://github.com/macports/upt-macports/commit/e187980cd6c7d0b4ff732aa99e6e7fbd9bdf47ab)

No worries, but you broke the tests - please fix ;)