-
Notifications
You must be signed in to change notification settings - Fork 124
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
HammerDBcli does not work correctly when added to PATH and called from other directory #597
Comments
Yes, HammerDB is intended to be run from the home directory which mostly to now has not caused a major issue and has not been a priority to change. You are welcome to submit a PR to enable calling by absolute path although some things to note first are that firstly modifications would be need to hammerdb, hammerdbcli, hammerdbws and agent on Linux and the equivalent .bat files on Windows including the additional mpstat.bat file on Windows. Also note that in the scripts the virtual users are also loading libraries, modules and packages from their relative directories e.g.
The virtual users run completely in parallel, so maintain their own separate environments. Therefore, probably the best solution if this is desired is the first suggestion to cd to the HammerDB directory if you are not already there first. Then it is least likely to break existing functionality. This will need to be done and checked for all the files mentioned above on both Linux and Windows. |
I submitted a draft, but I have still one doubt. Until we figure this out, I marked the PR as draft. |
And the same applies to all calls when using |
I think the answer to this is that someone is just as likely to start the agent with an absolute path as much as starting hammerdb, hammerdbcli etc - if we are changing the behaviour then it should be the same for everything. However it all works at the moment using the relative path, this is why we haven't done this before just in case something is inadvertently broken. |
The approach with HAMMERDB_HOME variable has a flaw too, ex. for the hammerdbcli updated with:
it works fine for the binaries used in bash, but in the TCL scripts, there are hardcoded paths to XML files (
Personally, I don't have enough TCL knowledge to tackle that, changing all these relative paths to be based on env variable (?) or variant absolute path. I also don't want to add another abstraction level to the executables that could work around the "changing directory before call" problem described before. I mean, I would use that locally as a workaround, but won't upstream that. In our previous discussion, Steve mentioned that there is an issue #383 that potentially can help with that issue, implementing wrapped executables. I think it would be worth waiting for that. |
I agree, the potential benefit of this change is outweighed by the amount of testing needed to ensure that everything continues to work. The preferred solution is to create a single file executable based on a virtual file system that can then be called from anywhere and this is planned in Tcl 8.7 without any external tools https://www.magicsplat.com/blog/tcl87-zipfs2/. It is also currently possible with Tcl 8.6 https://www.magicsplat.com/blog/starpack-example/ and we already includes Tclkits for the Bawt build process (to bootstrap the build before Tcl is built) and the new CloudTk deployment https://github.com/sm-shaw/CloudTk for running HammerDB as a web application. Therefore, there is a known methodology on how to do this. We will of course always keep the current packaging as a release where there is preference for direct access to the source in the build, however as Docker is becoming increasingly popular a single file executable is more appropriate for these builds. |
Is your feature request related to a problem? Please describe.
When adding HammerDB home to the PATH (on Linux), and calling hammerdbcli from a different directory, it throws an error, as it is using relative paths, like
./bin/tclsh8.6
1. Describe the solution you'd like
Add code to change the directory to HammerDB "home", in
hammerdb
,hammerdbcli
andhammerdbws
, right at the beginning of the scripts, in line 3. Example code:2. Describe alternatives you've considered
The other approach here can be to define in the same line 3 a variable called HAMMERDB_HOME, and change all relative paths to full paths, like
${HAMMERDB_HOME}/bin/tclsh8.6
instead of./bin/tclsh8.6
, but it would require more changes in the code.3. Describe alternatives you've considered
A third approach could be to suggest adding to the path both
${HAMMERDB_HOME}
and${HAMMERDB_HOME}/bin
and get rid of relative paths, usingtclsh8.6
instead of./bin/tclsh8.6
. But it can stop working when somebody has not added Hammerdb to PATH.Additional context
I can submit a PR on my own, but wanted to discuss a solution first.
The text was updated successfully, but these errors were encountered: