[MacPorts] #43202: macports.tcl: doesn't reset user if svn fails
#43202: macports.tcl: doesn't reset user if svn fails ----------------------+-------------------------------- Reporter: mojca@… | Owner: macports-tickets@… Type: defect | Status: new Priority: Normal | Milestone: Component: base | Version: 2.2.99 Keywords: haspatch | Port: ----------------------+-------------------------------- When svn fails to sync the ports (in my case that's because it doesn't accept the certificate), rsync continues to run as a local user. I'm attaching an example of a patch, but please check it carefully first as I don't have no experience messing up with base. Maybe something similar is needed at other places as well. (The patch is against trunk, but it would be nice to fix this for 2.3.0 as well.) -- Ticket URL: <https://trac.macports.org/ticket/43202> MacPorts <http://www.macports.org/> Ports system for OS X
#43202: macports.tcl: doesn't reset user if svn fails ----------------------+-------------------------------- Reporter: mojca@… | Owner: macports-tickets@… Type: defect | Status: new Priority: Normal | Milestone: Component: base | Version: 2.2.99 Resolution: | Keywords: haspatch Port: | ----------------------+-------------------------------- Comment (by mojca@…): One way to test this it to add one extra debug line with `geteuid` to `/opt/local/share/macports/Tcl/macports1.0/macports.tcl`: {{{ set needs_portindex 0 ui_info "Synchronizing local ports tree from $source" ui_info ">>> Hello me ([geteuid])" }}} and to replace {{{ system $svn_commandline }}} with {{{ system "/usr/bin/false" }}} -- Ticket URL: <https://trac.macports.org/ticket/43202#comment:1> MacPorts <http://www.macports.org/> Ports system for OS X
#43202: macports.tcl: doesn't reset user if svn fails ----------------------+---------------------- Reporter: mojca@… | Owner: cal@… Type: defect | Status: assigned Priority: Normal | Milestone: Component: base | Version: 2.2.99 Resolution: | Keywords: haspatch Port: | ----------------------+---------------------- Changes (by cal@…): * status: new => assigned * owner: macports-tickets@… => cal@… Comment: Thanks, that also was a problem for git. r118570. Should we backport that into 2.3 aswell? -- Ticket URL: <https://trac.macports.org/ticket/43202#comment:2> MacPorts <http://www.macports.org/> Ports system for OS X
#43202: macports.tcl: doesn't reset user if svn fails ----------------------+----------------------------- Reporter: mojca@… | Owner: cal@… Type: defect | Status: closed Priority: Normal | Milestone: MacPorts Future Component: base | Version: 2.2.99 Resolution: fixed | Keywords: haspatch Port: | ----------------------+----------------------------- Changes (by cal@…): * status: assigned => closed * cc: cal@… (removed) * resolution: => fixed * milestone: => MacPorts Future -- Ticket URL: <https://trac.macports.org/ticket/43202#comment:3> MacPorts <http://www.macports.org/> Ports system for OS X
#43202: macports.tcl: doesn't reset user if svn fails ----------------------+----------------------------- Reporter: mojca@… | Owner: cal@… Type: defect | Status: closed Priority: Normal | Milestone: MacPorts Future Component: base | Version: 2.2.99 Resolution: fixed | Keywords: haspatch Port: | ----------------------+----------------------------- Comment (by mojca@…): Thank you. As already mentioned it would be nice to see it in 2.3.0, but that's up to you or others to decide. I forgot to say earlier (and maybe I could write a patch as well), but it would probably be helpful to add a bit of extra `ui_debug` info. For example the following: {{{ set euid [geteuid] set egid [getegid] ui_debug "changing euid/egid - current euid: $euid - current egid: $egid" setegid [name_to_gid [file attributes $portdir -group]] seteuid [name_to_uid [file attributes $portdir -owner]] }}} would be more helpful if both initial and final UID and GID were reported (ideally also with username, not just number?). And it would probably be nice to cleary report the the change during reversal as well. An alternative would be to change all reports like {{{ ui_debug $svn_commandline }}} into something like {{{ ui_debug "Running '$svn_commandline' as [geteuid]" }}} but that would basically mean modifying debug all commands. Maybe that's too much. What do you think of moving `ui_debug $svn_commandline` from before the loop to just before it gets executed, so that UID change would be reported before it is printed out? -- Ticket URL: <https://trac.macports.org/ticket/43202#comment:4> MacPorts <http://www.macports.org/> Ports system for OS X
#43202: macports.tcl: doesn't reset user if svn fails ----------------------+----------------------------- Reporter: mojca@… | Owner: cal@… Type: defect | Status: closed Priority: Normal | Milestone: MacPorts Future Component: base | Version: 2.2.99 Resolution: fixed | Keywords: haspatch Port: | ----------------------+----------------------------- Comment (by cal@…): Replying to [comment:4 mojca@…]:
Thank you. As already mentioned it would be nice to see it in 2.3.0, but that's up to you or others to decide.
I'm not in any way more qualified to decide whether to backport that to 2.3.0 than you are. In fact, you could just go ahead and cherry-pick the change onto the 2.3 branch.
I forgot to say earlier (and maybe I could write a patch as well), but it would probably be helpful to add a bit of extra `ui_debug` info. For example the following: {{{ set euid [geteuid] set egid [getegid] ui_debug "changing euid/egid - current euid: $euid - current egid: $egid" setegid [name_to_gid [file attributes $portdir -group]] seteuid [name_to_uid [file attributes $portdir -owner]] }}} would be more helpful if both initial and final UID and GID were reported (ideally also with username, not just number?). And it would probably be nice to cleary report the the change during reversal as well.
Actually I was surprised the change was done manually. There are a few other places in MacPorts where euid/egid are set and those are IIRC capsuled in a proc that also prints some debugging info. Maybe re-using that code and making it print nicer messages would be the way to go? In fact, one could re-factor the whole thing as a block statement with appropriate error handling, like `registry::write { code }` does, something along the lines of {{{ runAsUser $uid $gid { # run svn command here } }}} that would set euid/egid, print debug messages, run the block of code with the changed privileges and restore the previous uid/gid when the block finishes or throws an error. -- Ticket URL: <https://trac.macports.org/ticket/43202#comment:5> MacPorts <http://www.macports.org/> Ports system for OS X
participants (1)
-
MacPorts