search

TEIC / Stylesheets

TEI XSL Stylesheets
231 stars 124 forks source link

Makefile creates installed transformtei with potentially incorrect path #552

Closed sydb closed 1 month ago

sydb commented 2 years ago

Per the comment preceding it, the “/usr” in the following line of the Makefile should read “${PREFIX}”.

    perl -p -i -e 's+^APPHOME=.*+APPHOME=/usr/share/xml/tei/stylesheet+' ${PREFIX}/bin/transformtei

This is currently line 136. See #551 for some discussion.

That said, I do not (at the moment) understand what $APPHOME vs $defaultAPPHOME is doing in the transformtei program. While this should probably be fixed either way, if we understood what was going on we might be able to skip this step entirely.

sydb commented 2 years ago

BTW, have to think about how to make sure that the “${PREFIX}” is expanded by the shell, not interpreted by Perl.

bleekere commented 1 year ago

Changed /usr to $PREFIX as proposed in the issue cf. 3bf32eb7ac02a275b1122cb5e3f8038f27bcd4c6. The $APPHOME vs $defaultAPPHOME may require additional discussion.

sydb commented 1 year ago

Oops. Looks like we never generated a PR for this and actually merged it with the dev branch. Re-opening for more thorough testing (we know the code generates the desired transformtei, but does that desired transformtei do the right thing?) and then creating PR.

ebeshero commented 1 month ago

Prodding @sydb to wrap this one up--should be easy?

sydb commented 1 month ago

No, certainly not during freeze. I am not sure what this “sydb” idiot was talking about back on 2023-04-04, but @bleekere (appropriately, IMHO) made this change to a branch which was never merged. So this change has never been tested, and looking at it, I think it might be wrong. (As alluded to earlier, one has to be very careful about whether make or perl resolves the ${} bit.) So this is a ticket to put on hold for a 2 weeks and play with after Balisage papers have been submitted. N.B. 3bf32eb7ac02a275b1122cb5e3f8038f27bcd4c6, the commit in which the change was made in the issue-552 branch.

ebeshero commented 1 month ago

@sydb I know! But you'll notice I set the next release milestone on it. I'm basically doing a sort of census of tickets that we'd marked for this milestone and making sure we don't lose track of these... So my comment on this ticket was fully intended for the 7.58 milestone!

sydb commented 1 month ago

OK. I have ascertained the ${PREFIX} is correctly being resolved by make before cmd is sent to perl. I did this by just testing. (It is not difficult to do, just issue make PREFIX=/path/to/place/you/want/installed/stylesheets installxsl using both the issue-552 branch and some other branch, and diff the resulting bin/transformtei files.) BUT, in so doing I discovered another problem — while make PREFIX=/tmp install works at least in this regard, make PREFIX=/tmp/ install fails in this regard because the resulting paths are /tmp//share/xml/tei/stylesheet/…. I am not going to address that on this ticket or the PR I am about to create, as it is an entirely separate (although related) problem.

sydb commented 1 month ago

Fix merged via #691.