Opened 9 years ago
Closed 9 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)
Change History (16)
Changed 9 years ago by MarcusCalhoun-Lopez (Marcus Calhoun-Lopez)
| Attachment: | patch-python-1.0.tcl.diff added |
|---|
comment:1 Changed 9 years ago by stromnov (Andrey Stromnov)
| Cc: | stromnov@… added |
|---|
comment:2 Changed 9 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 9 years ago by jmroot (Joshua Root)
| Cc: | jmr@… added |
|---|
comment:4 Changed 9 years ago by michaelld (Michael Dickens)
+1 looks good to me. -Always- best programming technique to keep phases separate.
comment:6 Changed 9 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 9 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 9 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 9 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:11 follow-up: 13 Changed 9 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.
comment:12 Changed 9 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 Changed 9 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 9 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 9 years ago by MarcusCalhoun-Lopez (Marcus Calhoun-Lopez)
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
Cc Me!