Fix temp file bug

[pcrockett]

I had fun with the previous PR and thought maybe I could try my hand at fixing the bug you found as well. Removed the temp file altogether.

[pcrockett]

Oh darn you beat me to it :)

[xonixx]

Interesting idea, but probably won't work. In past I spent long time trying to find a way to not rely on temp files.
I think the main idea why I implemented it like this is to support the Makesurefile like

A=Hello
@define B="$A world"
@goal default
  echo $B

The trick with temp file allows to capture in them the "calculated" values of @define-d vars.
The additional constraint I chose to implement is to allow running prelude code only once (for efficiency, since it can have heavy initialization code, like reading files, etc.). This prevents the naive idea of simply prepending the goal bodies with the whole prelude.

Things to check (for me):

1. Do we have proper test for this specific case with defines? Add if no.
2. Document these details in developer notes doc.

Upd. And apparently the above snippet doesn't work. One more bug found.

[pcrockett]

You're right, and on my first attempt I didn't think about that. However there were tests! https://github.com/xonixx/makesure/blob/main/tests/18_vars_priority.sh My second attempt is what you see here. I also believe we are still only executing the prelude once. Think of it this way: Instead of appending @define lines to a file and dot-sourcing the file for every goal, we are now saving the lines in memory and prepending them to every goal. So I actually think this may result in a performance improvement, because we're not forcing the shell to open and read the same file multiple times. (Assuming I correctly understand how it all works... A large assumption indeed.)

[xonixx]

Absolutely agree on performance aspect and wish to get rid of temp files.

However the test https://github.com/xonixx/makesure/blob/main/tests/18_vars_priority.sh doesn't capture the case in snippet above.

So your solution will work on case like

@define A="A"
@define B="B $A"

but won't with

A="$(curl some-url)"   # we don't want to expose A to goals
@define B="A=$A"       # but we do want to expose B

Because in your case DefinesCode won't capture A initialization.

[pcrockett]

Ah ok I understand. It looks like that use case has never been supported though. To verify, I created a failing test on the main branch here:

pcrockett@4265f22

So that does look like a separate issue. Do you think we should put this in its own separate issue? I have some thoughts about it, but we're starting to get off-topic for this PR.

[xonixx]

Yeah, it's funny, but this is how I designed initially and how I believed it really works.

Now this whole discussion made me to strongly reconsider the real necessity of the notion of prelude (the script that unconditionally runs before all goals).

It appears that if I myself didn't need to use it to the whole power, maybe it's useless after all.
Also it opens the possiblity to be abused or misused. What if users start writing huge codes in prelude, what implications will be?
Besides, why allow some arbitrary logic in prelude if this logically can be just anoter @goal initialized and any other goal can depend on it if need be.
Also the current operational behavior of @define-s is vague, such as even I, the author, am unsure how it really works.

This makes me think that we can just drop this feature and only allow plain static defines like

@define A 'a value'
@define B 'some othe value' # can not reference A

I need some time to analyze all pros and cons now. Definitelly this is in spirit of Worse is better which I strongly believe in.

This is definitelly a breaking change, but I doubt it hurts anyone, since the user base is still very small :-)

[pcrockett]

I think I agree. I will say I often do things like this: @define A_FILE_NAME="filename" @define A_FILE_PATH="path/to/${A_FILE_NAME}" However I have never done anything like this before: A_FILE_NAME="filename" @define A_FILE_PATH="path/to/${A_FILE_NAME}" Which makes me think that yes, the prelude is unnecessary. @goal, @lib, and @define have all the functionality you need, and encourage better organization.

[xonixx]

This is actually implemented in context of #84 and will be released soon.
Therefore closing this PR. @pcrockett thanks for your ideas!