Fix deleting duplicated rpaths

[isuruf]

No description provided.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.80%. Comparing base (43ad6b3) to head (9b59f36).

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #208      +/-   ##
==========================================
- Coverage   96.80%   96.80%   -0.01%     
==========================================
  Files          15       15              
  Lines        1285     1283       -2     
==========================================
- Hits         1244     1242       -2     
  Misses         41       41              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[HexDecimal]

There are performance implications to calling replace_signature multiple times. It is already called too many times for the same file in the current code.

Let delete_rpath take multiple paths. Counter can be useful to track when a parameter must be given multiple times. Actually just iterating over the paths one at a time is fine as long as replace_signature is called only once.

If a new function must be added then make it private: def _delete_rpath

Mind editing the docs for get_rpaths to clarify that duplicate rpaths may be returned?

[isuruf]

Sorry, didn't see your edit. Shall I revert the Counter changes?

[HexDecimal]

I still think _delete_rpath should take an iterable of paths to remove (paths: Iterable[str]). replace_signature and logging would be moved into _delete_rpath.

Then _remove_absolute_rpaths would be simplified to this:

_delete_rpath(
    filename,
    (rpath for rpath in get_rpaths(filename) if not _is_rpath_sanitary(rpath)),
)

[HexDecimal]

Close to finishing. This should also be mentioned as a fix in the changelog.

[HexDecimal]

Redundant since _delete_rpaths handles signing.

Suggested change

	if ad_hoc_sign:
	replace_signature(filename, "-")

[HexDecimal]

Less vague changelog item.

Suggested change

	- Fixes deleting duplicated rpaths when sanitize-rpaths option is turned on.
	[#208](https://github.com/matthew-brett/delocate/pull/208)
	- `--sanitize-rpaths` was not removing duplicate rpaths.
	[#208](https://github.com/matthew-brett/delocate/pull/208)

[isuruf]

Changed this to '--sanitize-rpaths was failing with duplicate rpaths.' as it was failing with an exception from subprocess.

[HexDecimal]

Latest commit mentioned but did not touch the redundant replace_signature call.

At this point I can handle the rest if you're finished.

[isuruf]

Thanks @HexDecimal for the reviews.

[isuruf]

@HexDecimal is there a release planned for delocate?

[HexDecimal]

I'm slightly hesitant with the recent changes and the changes I've been considering, but nothing is actually preventing a release it in its current state. I'll do a release now. I probably should've done one earlier.