Opened 3 months ago

Closed 3 months ago

Last modified 3 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) 3 months ago.
side-by-side comparison of strace output

Download all attachments as: .zip

Change History (6)

Changed 3 months ago by RJVB (René Bertin)

Attachment: screenshot.png added

side-by-side comparison of strace output

comment:1 Changed 3 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 3 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 3 months ago by RJVB (René Bertin)

"It works fine" for me too, but without the protection against multiple applications of the same patchfile. I wouldn't have filed this issue if I'd been better awake and had thought of double-checking the PortGroups ...

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".

Last edited 3 months ago by RJVB (René Bertin) (previous) (diff)

comment:4 Changed 3 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 3 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.