Opened 8 years ago

Closed 8 years ago

#49948 closed defect (fixed)

python-1.0.tcl: destroot.env not always modified

Reported by: MarcusCalhoun-Lopez (Marcus Calhoun-Lopez) Owned by: macports-tickets@…
Priority: Normal Milestone:
Component: ports Version: 2.3.4
Keywords: haspatch Cc: stromnov (Andrey Stromnov), jeremyhu (Jeremy Huddleston Sequoia), jmroot (Joshua Root), michaelld (Michael Dickens), ryandesign (Ryan Carsten Schmidt), petrrr
Port:

Description

destroot.env is modified in pre-build, which means that it will not be modified with the commands port build && port destroot.
Attached is a proposed solution.

Attachments (1)

patch-python-1.0.tcl.diff (2.6 KB) - added by MarcusCalhoun-Lopez (Marcus Calhoun-Lopez) 8 years ago.

Download all attachments as: .zip

Change History (16)

Changed 8 years ago by MarcusCalhoun-Lopez (Marcus Calhoun-Lopez)

Attachment: patch-python-1.0.tcl.diff added

comment:1 Changed 8 years ago by stromnov (Andrey Stromnov)

Cc: stromnov@… added

Cc Me!

comment:2 Changed 8 years ago by jmroot (Joshua Root)

Cc: jeremyhu@… added

r130565 was certainly incorrect to add the destroot stuff in pre-build. However I could never reproduce the problem that it supposedly solved.

comment:3 Changed 8 years ago by jmroot (Joshua Root)

Cc: jmr@… added

comment:4 Changed 8 years ago by michaelld (Michael Dickens)

+1 looks good to me. -Always- best programming technique to keep phases separate.

comment:5 Changed 8 years ago by michaelld (Michael Dickens)

Cc: michaelld@… added

Cc Me!

comment:6 Changed 8 years ago by jmroot (Joshua Root)

I'm inclined to simply remove the destroot.env unless someone can demonstrate a problem caused by that.

comment:7 Changed 8 years ago by ryandesign (Ryan Carsten Schmidt)

Cc: ryandesign@… added

Jeremy evidently thought there was a problem that r130565 fixed, so I'd err on the side of caution and fix it as Marcus proposed in his patch.

comment:8 Changed 8 years ago by michaelld (Michael Dickens)

I'm with Ryan here. Given then # of ports that this change will impact, I think we're all better off erring on the side of caution.

comment:9 Changed 8 years ago by jmroot (Joshua Root)

I tested the examples he gave at the time and the change did nothing. https://lists.macosforge.org/pipermail/macports-dev/2015-January/029391.html Adding a chunk of code that does nothing is a bad idea.

comment:10 Changed 8 years ago by petrrr

Cc: petr@… added

Cc Me!

comment:11 Changed 8 years ago by jeremyhu (Jeremy Huddleston Sequoia)

IIRC, having them different caused the destroot phase to *rebuild* the port with the new settings rather than just install what was previously built.

Last edited 8 years ago by jeremyhu (Jeremy Huddleston Sequoia) (previous) (diff)

comment:12 Changed 8 years ago by michaelld (Michael Dickens)

OK; yeah. What Jeremy writes sounds correct. Maybe we need duplicate build and destroot phase settings: both in both areas so that the builds are consistent no matter which phase one starts in? Redundant, yes; but, maybe better than the alternative. Hmmm ... Wishing whoever put that code in place also included a link to a ticket or something useful as to why it's there. Ah well; here we are now!

comment:13 in reply to:  11 Changed 8 years ago by jmroot (Joshua Root)

Replying to jeremyhu@…:

IIRC, having them different caused the destroot phase to *rebuild* the port with the new settings rather than just install what was previously built.

If you can reproduce that, fine. But I can't. See the link in comment:9.

comment:14 Changed 8 years ago by jeremyhu (Jeremy Huddleston Sequoia)

I was reproducing it myself. I don't have logs from that. If you want to remove it, go ahead, and if/when it crops back up, we can re-explore it.

comment:15 Changed 8 years ago by MarcusCalhoun-Lopez (Marcus Calhoun-Lopez)

Resolution: fixed
Status: newclosed

The proposed patch was applied in r143449.
To modify destroot.env the is now an option (r143452).
It is turned on by default to maintain current behavior.

py-numpy apparently has trouble if build.env and destroot.env are inconsistent (#34562).

Note: See TracTickets for help on using tickets.