Allow for simpler environmental variable creation

[Pocketkid2]

This is a change that @schafernc and I developed further after your comment here: #322 (comment)

We would really like a feature like this to be implemented because it would greatly simplify the process of adding environmental variables to the Makefile for flows such as F4PGA which at the present rely very heavily on them.

Let us know what your thoughts are on this matter. I figured it would be nice to pull this discussion out into a separate PR.

[olofk]

Yes! Great idea to do this as a separate PR. I still think this is a can of worms for two reasons.

1. As you have noticed, Windows uses set while other platforms uses export. But there are also many users of csh or derivatives that uses setenv instead. Not sure how we can detect that.
2. While Makefiles support setting env vars, I'm not sure that ninja files allow that. I think you can still do it with e.g. VAR=value <command> but there is no support for adding a bunch of export/set/setenv specifically.

I'm also highly sceptical towards the usage of all these env vars in the first place. Having dabbled a bit with symbiflow it seemed very much like they were only used to define variables to have all configuration in one place and not really used as env vars at all. In that case there is no point in having them at all in the generated makefile where we can just put in the final value directly.

At the same time, I do see that this functionality fills a gap and it will likely work for a majority of the cases, so I'm leaning towards pulling it in but without any guarantees that it will stay in the code base

[Pocketkid2]

2. While Makefiles support setting env vars, I'm not sure that ninja files allow that.

Can you tell me more about ninja and how it integrates with Edalize? I don't know anything about it at the moment

[Pocketkid2]

I'm also highly sceptical towards the usage of all these env vars in the first place. Having dabbled a bit with symbiflow it seemed very much like they were only used to define variables to have all configuration in one place and not really used as env vars at all. In that case there is no point in having them at all in the generated makefile where we can just put in the final value directly.

The real reason why we don't want to drop the environmental variables altogether is that they are heavily used inside the F4PGA/Symbiflow python scripts. I know we already discussed working those scripts out of the flow, but from the perspective of my project that's a pretty big task that's set as a long-term goal.

[Pocketkid2]

1. As you have noticed, Windows uses set while other platforms uses export. But there are also many users of csh or derivatives that uses setenv instead. Not sure how we can detect that.

I don't really have much experience with csh but I see your point. Are there a substantial number of users that use Edalize through it? Are environmental variables the primary difference that Edalize would have in csh?

[olofk]

I'm also highly sceptical towards the usage of all these env vars in the first place. Having dabbled a bit with symbiflow it seemed very much like they were only used to define variables to have all configuration in one place and not really used as env vars at all. In that case there is no point in having them at all in the generated makefile where we can just put in the final value directly.

The real reason why we don't want to drop the environmental variables altogether is that they are heavily used inside the F4PGA/Symbiflow python scripts. I know we already discussed working those scripts out of the flow, but from the perspective of my project that's a pretty big task that's set as a long-term goal.

Ok...passing all this stuff as env vars to the python scripts was slightly unexpected. It's kind of like only using global variables. But as you say, getting that cleaned up is probably best left as a separate project. With that said however, I would prefer if we could limit the scope of these env vars, so that in the makefiles we only pass the env vars that are actually used for each tool. A command-line in the makefile would then typically look like SOMEVAR=somevalue ANOTHERVAR=differentvalue python3 some_script.py.But are you sure they are that much used? I looked through the python scripts that I know are part of the flow (split_inouts.py, prjxray_create_ioplace.py, prjxray_create_place_constraints.py) and none of them use a single env var.

I do see a lot of them in the tcl files launched by yosys though (which is something I tried to clean up last year but gave up on because it was just too much spaghetti code), and in that case we would pass the relevant env vars when invoking yosys

[olofk]

1. As you have noticed, Windows uses set while other platforms uses export. But there are also many users of csh or derivatives that uses setenv instead. Not sure how we can detect that.

I don't really have much experience with csh but I see your point. Are there a substantial number of users that use Edalize through it? Are environmental variables the primary difference that Edalize would have in csh?

I think csh is the default on typical enterprise distributions like redhat but I'm not completely sure about that. I do know that many EDA tools supply both .sh and .csh scripts to handle both types of environments. But if both environments (I know sh-based ones like bash does) support launching programs with SOMEVAR=somevalue <program> then it's better to use that syntax instead

[olofk]

2. While Makefiles support setting env vars, I'm not sure that ninja files allow that.

Can you tell me more about ninja and how it integrates with Edalize? I don't know anything about it at the moment

Ninja is a modern light-weight build system. Perhaps the main benefit of using that for Edalize is that it is much easier to get going on non-linux platforms. Longer term, we want to support a wider array of build orchestration tools. We could e.g. automatically generate CI pipeline scripts or distribute different steps of the build to different machines using LSF and SLURM instead of a single machine running a makefile. Right now there's only a makefile backend for the EdaCommand class, but I want to be a bit conservative so that we don't add a lot of features that make it harder to port to other backends in the future.

[Pocketkid2]

You

Ok...passing all this stuff as env vars to the python scripts was slightly unexpected. It's kind of like only using global variables. But as you say, getting that cleaned up is probably best left as a separate project. With that said however, I would prefer if we could limit the scope of these env vars, so that in the makefiles we only pass the env vars that are actually used for each tool. A command-line in the makefile would then typically look like SOMEVAR=somevalue ANOTHERVAR=differentvalue python3 some_script.py.But are you sure they are that much used? I looked through the python scripts that I know are part of the flow (split_inouts.py, prjxray_create_ioplace.py, prjxray_create_place_constraints.py) and none of them use a single env var.

I do see a lot of them in the tcl files launched by yosys though (which is something I tried to clean up last year but gave up on because it was just too much spaghetti code), and in that case we would pass the relevant env vars when invoking yosys

You're right, I keep getting the python scripts confused with the TCL scripts. For yosys do we just pass in the variables like you show above? Maybe we should make an option for all tools at the edatool level that allows you to pass in variables, and then those variables are put in front of the command as shown which would then solve the problem for yosys and any other tools that may need variables.

[Pocketkid2]

I think csh is the default on typical enterprise distributions like redhat but I'm not completely sure about that. I do know that many EDA tools supply both .sh and .csh scripts to handle both types of environments. But if both environments (I know sh-based ones like bash does) support launching programs with SOMEVAR=somevalue <program> then it's better to use that syntax instead

Yeah, based on your above comment I think this might be the right way to go forward, but maybe we should do it at the Edatool level?

[Pocketkid2]

Ninja is a modern light-weight build system. Perhaps the main benefit of using that for Edalize is that it is much easier to get going on non-linux platforms. Longer term, we want to support a wider array of build orchestration tools. We could e.g. automatically generate CI pipeline scripts or distribute different steps of the build to different machines using LSF and SLURM instead of a single machine running a makefile. Right now there's only a makefile backend for the EdaCommand class, but I want to be a bit conservative so that we don't add a lot of features that make it harder to port to other backends in the future.

I see, so Ninja is more like an alternative to Makefiles. If Ninja has its own backend then will what we do here really affect it that much?

[Pocketkid2]

@olofk I'm going to send in a PR with Yosys modification and see if we can supplant this PR with that.

[olofk]

Ok...passing all this stuff as env vars to the python scripts was slightly unexpected. It's kind of like only using global variables. But as you say, getting that cleaned up is probably best left as a separate project. With that said however, I would prefer if we could limit the scope of these env vars, so that in the makefiles we only pass the env vars that are actually used for each tool. A command-line in the makefile would then typically look like SOMEVAR=somevalue ANOTHERVAR=differentvalue python3 some_script.py.But are you sure they are that much used? I looked through the python scripts that I know are part of the flow (split_inouts.py, prjxray_create_ioplace.py, prjxray_create_place_constraints.py) and none of them use a single env var.
I do see a lot of them in the tcl files launched by yosys though (which is something I tried to clean up last year but gave up on because it was just too much spaghetti code), and in that case we would pass the relevant env vars when invoking yosys

You're right, I keep getting the python scripts confused with the TCL scripts. For yosys do we just pass in the variables like you show above? Maybe we should make an option for all tools at the edatool level that allows you to pass in variables, and then those variables are put in front of the command as shown which would then solve the problem for yosys and any other tools that may need variables.

I did some testing and it seems like env VAR1=somevalue VAR2=someothervalue <command> works for both sh and csh, so let's go with that. I suggest to update the add function in the EdaCommands class so that you can pass in environment variables for specific commands. Looks like this is going to be a shit show on Windows (https://superuser.com/questions/223104/setting-and-using-variable-within-same-command-line-in-windows-cmd-exe) but let's worry about that later until we can have someone who can actually test it for us. That would, as you suggest, then be available for every tool (since we last spoke I have encountered two separate places where we most likely need to set env vars) and also be the same whether we use make or ninja for flow orchestration.

[Pocketkid2]

@olofk Take a look at the new commit. I can remove the other add_env_var stuff if you like, or maybe we can leave it in as a backup option.

I've also merged this branch into the Yosys change PR and updated that to reflect this new change (see #335)

[olofk]

Yes. Let's keep the add_env_var function for now to remember that we eventually need to figure out what to do on windows as well.

Thank you for your contribution. Squashed and merged!