Opened 2 months ago

Closed 2 months ago

Last modified 2 months ago

#70462 closed defect (duplicate)

rust 1.0 portgroup includes muniversal 1.1 portgroup which replaces patch_main

Reported by: RJVB (René Bertin) Owned by:
Priority: Normal Milestone:
Component: ports Version:
Keywords: Cc:
Port:

Description

I've stumbled across a very curious, weird even side-effect of loading the rust PortGroup. (I've picked "defect" for this ticket because it seemed the least inappropriate.)

It seems to replace the patch_main procedure that applies a port's patchfiles.

I noticed this because I have a local patch to this function that records each successfully applied patch in the statefile, and uses check_statefile to verify if a patch has already been applied before attempting to apply it (again). This is very practical during port maintenance as I can evoke port patch repeatedly until all patchfiles have been refactored.

At first I thought of some kind of race condition or caching of the statefile but it is in fact as if the portpatch::patch_main from $prefix/libexec/macports/lib/port1.0/portpatch.tcl isn't being called at all.

For instance, if I add a Tcl throw statement to the function in that file and then invoke port -nv patch on a port that includes the rust PG and has patchfiles:

  • the command completes correctly with the unmodified Portfile
  • the requested error will be thrown if I comment out PortGroup rust 1.0 and every line that depends on it in the Portfile

That really suggests an alternative patch_main procedure is being called, but I fail to find any evidence of its existence, nor how/why it would be overloaded in the rust PG.

I'm seeing this issue on both my Mac and with the ported-to-Linux version on the system I'm typing this message on.

Any thoughts, please?

Below are some subtle differences in the output from strace -e trace,open,close,read,write,mkdir of the port -nv patch command *on linux*. One can see that the additional calls to check_statefile and write_statefile are not being executed, but there's also a very strange difference in the last write to fd=6 (the logfile). In the "without rust" case, that write command is the "Starting logging" message that appears at the beginning of the logfile: it beats me why this would not show up at the start in the strace output (it doesn't either in the "with rust" case, but earlier). I'll attach a screenshot of a side-by-side comparison.

*Without including the rust PG:

write(7, "target: org.macports.extract\n", 29) = 29
close(7)                                = 0
open("/opt/local/site-ports/devel/rustup/Portfile", O_RDONLY) = 7
read(7, "# -*- coding: utf-8; mode: tcl; "..., 8192) = 3128
read(7, "", 8192)                       = 0
close(7)                                = 0
open("/path/to/.macports.rustup.state", O_RDWR|O_CREAT, 0666) = 7
read(7, "version: 2\nchecksum: d314d1855f1"..., 4096) = 172
read(7, "", 4096)                       = 0
write(1, "--->  Applying patches to rustup"..., 38) = 38
read(7, "version: 2\nchecksum: d314d1855f1"..., 4096) = 172
read(7, "", 4096)                       = 0
write(1, "--->  Applying patch-allow-not-c"..., 55) = 55
close(9)                                = 0
read(8, "patching file src/toolchain/tool"..., 4096) = 41
write(1, "patching file src/toolchain/tool"..., 42) = 42
read(8, "", 4096)                       = 0
--- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=31472, si_status=0, si_utime=0, si_stime=0} ---
close(8)                                = 0
close(8)                                = -1 EBADF (Bad file descriptor)
read(7, "version: 2\nchecksum: d314d1855f1"..., 4096) = 172
read(7, "", 4096)                       = 0
write(7, "patch: patch-allow-not-calling-r"..., 42) = 42
read(7, "version: 2\nchecksum: d314d1855f1"..., 4096) = 214
read(7, "", 4096)                       = 0
write(7, "target: org.macports.patch\n", 27) = 27
close(7)                                = 0
write(6, "version:1\n:debug:main Starting l"..., 3502) = 3502
close(6)                                = 0
open("/opt/local/var/lnxports/last_reclaim", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/opt/local/var/lnxports/pingtimes", O_WRONLY|O_CREAT|O_TRUNC, 0666) = 6
close(5)                                = 0
close(4)                                = 0
close(3)                                = 0
close(6)                                = 0
+++ exited with 0 +++

*With the rust PG:

write(7, "target: org.macports.extract\n", 29) = 29
close(7)                                = 0
open("/opt/local/site-ports/devel/rustup/Portfile", O_RDONLY) = 7
read(7, "# -*- coding: utf-8; mode: tcl; "..., 8192) = 3126
read(7, "", 8192)                       = 0
close(7)                                = 0
open("/path/to/.macports.rustup.state", O_RDWR|O_CREAT, 0666) = 7
read(7, "version: 2\nchecksum: b65834cd86b"..., 4096) = 172
read(7, "", 4096)                       = 0
write(1, "--->  Applying patches to rustup"..., 34) = 34
write(1, "--->  Applying patch-allow-not-c"..., 51) = 51
close(9)                                = 0
read(8, "patching file src/toolchain/tool"..., 4096) = 41
write(1, "patching file src/toolchain/tool"..., 42) = 42
read(8, "", 4096)                       = 0
--- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=31691, si_status=0, si_utime=1, si_stime=0} ---
close(8)                                = 0
close(8)                                = -1 EBADF (Bad file descriptor)
read(7, "version: 2\nchecksum: b65834cd86b"..., 4096) = 172
read(7, "", 4096)                       = 0
write(7, "target: org.macports.patch\n", 27) = 27
close(7)                                = 0
write(6, "ir /opt/local/var/lnxports/build"..., 3329) = 3329
close(6)                                = 0
open("/opt/local/var/lnxports/last_reclaim", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/opt/local/var/lnxports/pingtimes", O_WRONLY|O_CREAT|O_TRUNC, 0666) = 6
close(5)                                = 0
close(4)                                = 0
close(3)                                = 0
close(6)                                = 0
+++ exited with 0 +++

Attachments (1)

screenshot.png (236.4 KB) - added by RJVB (René Bertin) 2 months ago.
side-by-side comparison of strace output

Download all attachments as: .zip

Change History (6)

Changed 2 months ago by RJVB (René Bertin)

Attachment: screenshot.png added

side-by-side comparison of strace output

comment:1 Changed 2 months ago by RJVB (René Bertin)

#$@#)()Y@#$

Somehow I missed the fact that the muniversal-1.1 PG overloads the patch_main function, grrrr! I'm sure I scanned my PG directory for "patch_main" so I guess I need to start wearing reading glasses while coding.

Sorry for the noise..

comment:2 Changed 2 months ago by ryandesign (Ryan Carsten Schmidt)

Component: baseports
Resolution: duplicate
Status: newclosed
Summary: Curious side-effect of loading the rust PG?!rust 1.0 portgroup includes muniversal 1.1 portgroup which replaces patch_main

Right, the rust 1.0 portgroup includes the muniversal 1.1 portgroup which replaced patch_main. Which works fine for everyone except you because you have local modifications to patch_main that muniversal's modified version doesn't have. So let's get those modifications into base and the muniversal portgroup. That's #46927.

comment:3 Changed 2 months ago by RJVB (René Bertin)

"It works fine" for me too, but without the protection against multiple applications of the same patchfile.

I've attached an equivalent patchfile for the PG to #46927 . It must open the statefile once more as I don't think one can obtain the file descriptor. As far as I'm concerned those patches can be merged as-is directly by anyone, without PR.

I seemed to recall that the file was closed after each modification but that was wishful thinking or no longer the case. It would be better probably but moot if & when muniversal or its patch_main tweak gets merged into "base".

Version 0, edited 2 months ago by RJVB (René Bertin) (next)

comment:4 Changed 2 months ago by RJVB (René Bertin)

BTW, is it the idea that ports might need arch-dependent patches even when not building +universal (and that this is preferable over writing patches that introduce arch-specific/conditional code)?

comment:5 in reply to:  2 Changed 2 months ago by RJVB (René Bertin)

Replying to ryandesign:

So let's get those modifications into base and the muniversal portgroup. That's #46927.

https://github.com/macports/macports-base/pull/345

Note: See TracTickets for help on using tickets.