Changed setup.py's extend_user_env_posix to not rewrite every time.

[WesleyAC]

I changed setup.py's extend_user_env_posix function. This change means 2 things:

1.) extend_user_env_posix writes to ~/.bashrc, not ~/.profile. It's more widly used and doesn't require a new login, just a new virtual terminal. 2.) extend_user_env_posix will check if it needs to write to ~/.bashrc, and only write if ~/.bashrc doesn't include an export statment for the current directory.

[WesleyAC]

In testing this, I noticed that only the most recent install of TRSP will be usable, because only the last export statment will take effect.

However, I don't know how to fix this, so it'll have to do.

[lramati]

Why not check against a version number instead of against the export statement?

[WesleyAC]

@firerogue What do you mean? I don't see any version numbers.

[lramati]

I misunderstood the problem. I thought it was with upgrading, whereas its really with multiple installs

[mrhmouse]

I use zsh, not bash, so this just creates a ~/.bashrc file in my home directory that I now have to merge manually. bash is usually what a user has as their shell, but not always.

Also, checking for the export line is a nice idea, but it doesn't cover every case. Imagine the scenario where either the environment is set up in a separate file (I do this, as do many people, to keep my ~/.profile small), or where the export line doesn't exactly match your template text. In these cases, the extra export is still written to ~/.bashrc. I'm not sure what good, general-case fix for this would be.

It's a tiny bit more time consuming, but perhaps we could do something like the following:

• Ask the user whether they want their $PATH updated in the first place.
• Get a list of some of the common files used to store the $PATH.
• Prompt the user to choose one of these files to add the export line to
• Allow the user to enter a filename if no choice is satisfactory

[WesleyAC]

@mrhmouse I made it ask about the location of the env file.

[WesleyAC]

Alright, I got around to fixing this after an embarrassingly long time.

As it turns out, I use zsh now so I can verify that it works with zsh!

[WesleyAC]

Wait, it turns out that I didn't test it enough. $RED_SPIDER_ROOT is added to $PATH, but rsshell is in $RED_SPIDER_ROOT/extbin, thus typing rsshell won't do anything, but extbin/rsshell will launch the shell.

I am really confused. Can someone explain this to me? And specifically explain what setup_user_env_posix is supposed to do in this situation. It seems like when it's called, we should add $RED_SPIDER_ROOT/extbin to $PATH, but that currently, it does what it says on the box.

[lramati]

God I haven't looked at this codebase in forever. I still haven't, so take what I say with a grain of salt, but iirc, the project's binaries folder(s) should be added to the environment's execution path, but the project's root is where you are sent when rsshell starts up

[jgonggrijp]

/extbin is permanently appended to the PATH by the setup script, /bin is temporally prepended every time you run rsshell. Does that clear up your confusion, @WesleyAC ?

[WesleyAC]

@jgonggrijp It seems that the script to append /extbin to PATH runs set_user_env, thus set_user_env gets run twice the first time...

The only way I can think of to fix this is to make a global user_env_path variable, but that seems like a bit of a hack. Any advice?

[jgonggrijp]

I'll have a look. Is that a change that you introduced (in order to solve another problem), or was it running twice already before you started working on it?

[WesleyAC]

@jgonggrijp It was already happening, but the asking about the env file location makes it an issue.

[jgonggrijp]

@WesleyAC I close-read the code (of setup.py), but I'm not seeing any reason why /extbin would be appended to the PATH twice, or why any of the enclosing code would be run twice. As far as I can tell all parts of the chain are invoked only once and nothing appears in a loop. I also don't see any recursion going on. Could you please explain to me how

It seems that the script to append /extbin to PATH runs set_user_env, thus set_user_env gets run twice the first time...

?

[WesleyAC]

extbin gets appended once, but the program runs extend_user_env_posix twice: lines 50 and 189 of my most recent commit. This prompts the user for input twice, which I think it should not, as it already has the information it needs.

[jgonggrijp]

Ah, I see what you mean. I think a global variable, which you suggested earlier, is fine in this case. Set the (global) envdir to None initially and only run the code to find the right value if it evaluates false. Perhaps move those lines into a separate function. I can also push some commits that implement these suggestions if you want.

It's a bit of a hack, but the entire setup script is a bit of a hack. Don't worry about it too much.

[lramati]

So... I've recently decided to do this the long and hard way by unrolling the entire setup script into one long main function, cleaning up the code, correcting the logic, and then breaking it back down into functions. So far i've found a few issues, but I'll only mention the two that are most relevant to this issue. A. Why does the first call to set_user_env happen in the first place? and why is it called with 'o', as in overwrite? why are we overwriting the path in the registry / ~/.profile? B. Why don't we automatically write to .profile if every shell accesses it eventually? This is besides the fact that the way its currently set up, if I ever try zsh even once, I'll have a .zshrc file, and the codebase will automatically assume I actively use it. as a partial solution to B, this Stack Overflow page, answer 2 suggests os.getenv('shell'). Answer 3 indicates some pitfalls, but I dont think we have to worry about any of them...

[lramati]

furthermore, this Linux Questions page suggests that not all shells use export to set environment variables....

[jgonggrijp]

A. Why does the first call to set_user_env happen in the first place?

Because rsshell needs to know the location of the project root directory.

and why is it called with 'o', as in overwrite? why are we overwriting the path in the registry / ~/.profile?

Because RED_SPIDER_ROOT is a single directory, not a set of possible directories. So if you set it, the old value obviously has to be removed.

B. Why don't we automatically write to .profile if every shell accesses it eventually? This is besides the fact that the way its currently set up, if I ever try zsh even once, I'll have a .zshrc file, and the codebase will automatically assume I actively use it.

Apparently, because some people have strong preferences on which configuration file should be used. This is a good question though. A more lightweight approach might be to ask whether the user is OK with ~/.profile or whether they prefer some other file, and in both cases check whether the chosen file already contains the path.

[lramati]

Oooh. Ok. I see how it works now. Since you don't need it in place for the install half of the script, why not stick it in the else branch of the if installing? Like if install: install(); else: set_user_env()?

[jgonggrijp]

You do need it when installing, because all installing (mostly copying from /src to /bin) happens relative to the project root. On top of that, if a user wants to set RED_SPIDER_ROOT, they probably also want to install.

[lramati]

But that's what the os.chdir() call is for. And it changes the directory based on the in-program variable, not the shell's environment variable

[jgonggrijp]

Yes, but the reverse is also true: if the user wants to install the programs, then they also want to be able to use them. And that's only possible if RED_SPIDER_ROOT is permanently saved, because rsshell relies on that.

[jgonggrijp]

@WesleyAC: what is your current stance towards your work on setup.py?

As far as I'm concerned your work is still valuable, despite the fact that firerogue is working on a complete rewrite of setup.py.

[WesleyAC]

@jgonggrijp Looking at this code it seems like refactoring would probably be a good idea, even if it's just separating it into a file per OS, it would make it a lot mare readable. Since @firerogue is working on a rewrite I'd want to get his ETA for the rewritten setup.py before I commit to work on this, but if it's going to be a while, I'll finish this pull request up.

[jgonggrijp]

@WesleyAC Since the amount of work that remains for you effort seems much smaller (to me) than the complete rewrite that firerogue just started out working on, I think it is more efficient if firerogue can build on your results than the other way round. Just my two cents.

[WesleyAC]

@jgonggrijp Well, the amount of work is the same, just who does it is different, so it really depends on if @firerogue has already started or not.

[lramati]

(Sorry about length! There's a tl;dr at the bottom!) I've only just begun my project, since I'm trying to simultaneously figure out all the bigger issues I've pointed out, rewrite the thing from scratch, and implement a bunch of the features we were hoping to eventually get around to writing. I think it makes the most sense for you to get setup.py to the point where it'll at least install a first and only instance properly, and then either do or don't help me get setup2.py to the point where its the shiny finished product we wanted on day one but couldn't figure out how to write, with all the bells and whistles. Either way, I think my next block of work for setup2.py will be to write out the flow of the program in comments so that I don't have to keep trying to remember what I'm up to when I come back to it after a week of inactivity, and also make it easier for others to collaborate with me.

• whew. Sorry for the insanely long response to that rather simple question. tl;dr: I'm nowhere near done, so setup.py is more important, but imo best course of action is making setup.py functional for the simplest use-cases, and then developing setup2.py properly from the ground up

[WesleyAC]

Currently setup.py works for the simplest of use cases, so I'll leave it be, as I don't think that this is important enough to warrant more work while setup2.py is being built.

[jgonggrijp]

It's not what I would do, but I respect your decision.