Perl-Toolchain-Gang/ExtUtils-Manifest

Bug with manicopy linking files instead of copying them when making distdir

Opened this issue · 8 comments

When executing the Makefile command make distdir, this calls the command create_distdir which uses ExtUtils::Manifest manicopy. It goes on to create a distribution directory, and for some obscure reason, the module files within this distribution directory seem linked to their originals. Indeed, when modifying the copied module, it also modifies the original one. Reverting the change made in the original also modifies the copy! It is as if the copy was not copied but hard linked to the original.

I doubt this should be the intended outcome if the PREOP option is set, because the procedure would modify both files.

To replicate the bug, take a perl distribution, open its main module in an IDE, then type perl Makefile.PL && make distdir, then edit the equivalent copied module file, and make some changes (e.g. change the version number), and you will see the same change also occurring in the original file.

Update:
This is not related to MacOS and is actually more related to ExtUtils::MakeMaker that calls ExtUtils::Manifest.

haarg commented

You can pass MakeMaker the option dist => { DIST_CP => 'cp' } to avoid this behavior.

You can pass MakeMaker the option dist => { DIST_CP => 'cp' } to avoid this behavior.

Thank you. I saw that. I think the default option should be cp if PREOP is used, otherwise there is a certain likelihood of problem like mine occurring.

I do not even understand why linking to files rather than plain copy.

Leont commented

Thank you. I saw that. I think the default option should be cp if PREOP is used, otherwise there is a certain likelihood of problem like mine occurring.

That could make sense, but I'm not sure it's really worth it for such a niche thing.

That could make sense, but I'm not sure it's really worth it for such a niche thing.

That's one perspective. Another is the risk-impact, i.e. the damage this has the potential of doing. But, I can understand if it is difficult to code this consideration.

Leont commented

Patches welcome?

Patches welcome?

Sure. Please find below my proposal. However, please note that I am not familiar with the code of this distribution, so there may need more adjustments.

*** /tmp/ExtUtils-MakeMaker-7.70/lib/ExtUtils//MM_Unix.pm	Sun Mar 26 22:13:13 2023
--- /tmp/MM_Unix-7.71.pm	Wed Aug 23 07:14:19 2023
***************
*** 633,639 ****

      $self->{CI}       ||= 'ci -u';
      $self->{RCS_LABEL}||= 'rcs -Nv$(VERSION_SYM): -q';
!     $self->{DIST_CP}  ||= 'best';
      $self->{DIST_DEFAULT} ||= 'tardist';

      ($self->{DISTNAME} = $self->{NAME}) =~ s{::}{-}g unless $self->{DISTNAME};
--- 633,639 ----

      $self->{CI}       ||= 'ci -u';
      $self->{RCS_LABEL}||= 'rcs -Nv$(VERSION_SYM): -q';
!     $self->{DIST_CP}  ||= ( $self->{PREOP} eq '$(NOECHO) $(NOOP)' ? 'best' : 'cp' );
      $self->{DIST_DEFAULT} ||= 'tardist';

      ($self->{DISTNAME} = $self->{NAME}) =~ s{::}{-}g unless $self->{DISTNAME};

haarg commented

Pretty sure that won't work, because PREOP is an argument to dist and doesn't store its value in $self.

I'm not sure that providing a PREOP option is really a useful signal for if the files in the dist dir are going to be modified. Most users use it to generate a README file. I can't tell what your use case is because you don't ship your cleanup script nor have it in your repositories.

You could also have your script modify the files by generating new files and renaming them over the existing files, rather than editing them in place.

Pretty sure that won't work, because PREOP is an argument to dist and doesn't store its value in $self.

I just copied what was in the original module...

You could also have your script modify the files by generating new files and renaming them over the existing files, rather than editing them in place.

But why would I go to such trouble, when the advertised PREOP option allows me to change the files before the operation?