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

Qucs uses QUCSDIR unconditionally #627

Closed
guitorri opened this issue Nov 30, 2016 · 7 comments
Closed

Qucs uses QUCSDIR unconditionally #627

guitorri opened this issue Nov 30, 2016 · 7 comments
Assignees
Labels
Milestone

Comments

@guitorri
Copy link
Member

From #353.

It is/will be a common situation that a Windows user, which has previously installed Qucs, uses the portable version without uninstalling the old version first. The result will be that some/many things will be taken from the old version, as seen above. IMHO, the portable version should have a qucs.bat file in the base directory that unsets QUCSDIR and then launches Qucs. And/or even better a startup message that inform the user that QUCSDIR is set and this should not normally be.

I didn't look at the installer but I think it should make sure that QUCSDIR is either not set (best) or set to the newest Qucs (but this may break the old install).

@guitorri
Copy link
Member Author

Right, I still have to push the modifications I did for the portable version.
But it does set QUCSDIR in the beginning of the qucs-portable.bat (can rename to qucs.bat as you say):

Rem   Get the location where the batch file resides.
Rem   Note: QUCSDIR ends with a backslash
set QUCSDIR=%~dp0

I did not prepare any Windows installers for the -rc3, but they still set QUCSDIR system wide... (the very reason why I am refraining from doing them)...

@in3otd
Copy link
Contributor

in3otd commented Dec 1, 2016

I think we should move away from QUCSDIR (didn't we do quite some work in the past for this very same purpose?); IIRC, on Linux is normally not set, except if the user explicitly sets it (also on Mac, I guess?), so the same should be on Windows. Just unset QUCSDIR in the .bat file, so Qucs will run in the same way as commonly done in the other platforms.

Can the Windows installer (if you plan to do one) be used to remove a environment variable? Will it ask to uninstall the old Qucs version first?

OTOH, as mentioned above I think we should really have a startup check that informs the user if QUCSDIR is set (or, at least, set to a different value than the actual app bin path) and a message that inform the user that it should not normally be.

@felix-salfelder
Copy link
Member

need to understand what QUCSDIR is trying to implement. (then get rid of it.)

evidently it was reinventing PATH, and that's trivial to fix. but what else?

startup check that informs the user if QUCSDIR is

really? QUCSDIR is (supposed to be) a user variable. so a user should not set it, unless required.

try $ PYTHONPATH=/tmp python. it does not "inform the user", and i think that's the only sensible thing to do. but sure, put whatever you like in a startup script...

@in3otd
Copy link
Contributor

in3otd commented Dec 1, 2016

need to understand what QUCSDIR is trying to implement.

yep, I think we already tried to understand why that variable exists and did not find a unique explanation; my impression is that different people (ab)used it for different purposes.
My understanding is that currently setting QUCSDIR is not needed to have Qucs properly working.

QUCSDIR is (supposed to be) a user variable. so a user should not set it, unless required.

I'll tend to agree, especially for the Linux world but for Windows we have a problem: in the past the Qucs Windows installer set the QUCSDIR environment variable, so now every Windows user has this variable set, and very likely the user does not know/remember that this variable is set.
Now, when ey tries the current Qucs "portable" version, the QUCSDIR variable will point to the old installed version and so things may/will not work as they should. This is why I wanted to explicitly inform the user that "someone" has previously set QUCSDIR for em and this may not be what em wanted.

@felix-salfelder
Copy link
Member

contrib/qucs_on_windows_startup_wrapper_with_some_workarounds_that_are_needed_due_to_misconceptional_use_of_environment_variables_in_the_past.sh... ?

or just qowswwswtandtmuoevitp.sh? qow_tp.sh?

yes, i see what you mean, and i'd prefer not to congest the code for this sort of stuff, main.cpp and qucs.cpp is full of mislocated cruft already ...

@guitorri guitorri added this to the 0.0.19 milestone Dec 1, 2016
@guitorri guitorri self-assigned this Dec 1, 2016
@guitorri
Copy link
Member Author

guitorri commented Dec 1, 2016

For Qucs standalone the variable QUCSDIR should not be needed. If it is set it will potentially be mixing up things with older installations. A startup wrapper that unset QUCSDIR and warns the user (deprecation warning) should be enough to keep old and new installations side by side.

From now on, even the installer might use a wrapper and avoid littering the host system with environment variables.

After this release we should focus on using the PATH how it is supposed to be used.

A few things still use the QUCSDIR variable to add QUCSDIR /bin to the PATH to find qucsconv:
https://github.com/Qucs/qucs/blob/release-0.0.19/qucs/qucs/qucsdigi.bat#L48
https://github.com/Qucs/qucs/blob/release-0.0.19/qucs/qucs/qucsveri.bat#L42

By the way, BINDIR is being passed but not used:
https://github.com/Qucs/qucs/blob/release-0.0.19/qucs/qucs/qucsdigi.bat#L30
https://github.com/Qucs/qucs/blob/release-0.0.19/qucs/qucs/qucsveri.bat#L30

BINDIR is used on the Linux wrappers to call $BINDIR/qucsconv
https://github.com/Qucs/qucs/blob/release-0.0.19/qucs/qucs/qucsdigi#L98

On Linux, it just expect iverilog to be on the PATH.
https://github.com/Qucs/qucs/blob/release-0.0.19/qucs/qucs/qucsveri#L60

This one sets the PATH in a similar way, but nothing from QUCSDIR/bin is used later on.
https://github.com/Qucs/qucs/blob/release-0.0.19/qucs/qucs/qucsdigilib.bat#L45

I will update the wrapper to do the following, I will simplify/remove things if possible.

  • check, warn user, unset QUCSDIR
  • add [QUCS_PREFIX]/bin into the PATH
  • set FREEDHL, add FREEDHL/bin into the PATH
  • add MINGWDIR/bin into the PATH
  • set ADMSXMLBINDIR (it is now by default at QUCSDIR/bin)

@guitorri
Copy link
Member Author

Should be fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants