Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Makefile creates installed transformtei with potentially incorrect path #552

Closed
sydb opened this issue May 28, 2022 · 8 comments
Closed
Assignees
Labels
resp: council target: build Concerning the build process, e.g. the Makefile or build.xml type: bug A bug report.

Comments

@sydb
Copy link
Member

sydb commented May 28, 2022

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 sydb added type: bug A bug report. resp: council target: build Concerning the build process, e.g. the Makefile or build.xml labels May 28, 2022
@sydb sydb added this to the Release 7.54.0 milestone May 28, 2022
@sydb
Copy link
Member Author

sydb commented May 29, 2022

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

@bleekere bleekere self-assigned this Jun 1, 2022
bleekere added a commit that referenced this issue Sep 28, 2022
@bleekere
Copy link
Contributor

bleekere commented Sep 28, 2022

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

@sydb
Copy link
Member Author

sydb commented Apr 4, 2023

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.

@sydb sydb reopened this Apr 4, 2023
@sydb sydb modified the milestones: Release 7.55.0, Release 7.56.0 Apr 4, 2023
@HelenaSabel HelenaSabel modified the milestones: Release 7.56.0, Release 7.57.0 Nov 13, 2023
@ebeshero
Copy link
Member

ebeshero commented Jul 6, 2024

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

@sydb
Copy link
Member Author

sydb commented Jul 6, 2024

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. 3bf32eb, the commit in which the change was made in the issue-552 branch.

@ebeshero
Copy link
Member

ebeshero commented Jul 6, 2024

@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
Copy link
Member Author

sydb commented Jul 18, 2024

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 added a commit that referenced this issue Jul 18, 2024
* Changed  to  in Makefile #552

* Forgot to delete no-longer needed comment :-)

---------

Co-authored-by: Elli Bleeker <[email protected]>
@sydb
Copy link
Member Author

sydb commented Jul 18, 2024

Fix merged via #691.

@sydb sydb closed this as completed Jul 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
resp: council target: build Concerning the build process, e.g. the Makefile or build.xml type: bug A bug report.
Projects
None yet
Development

No branches or pull requests

5 participants