search

lnls-fac / apsuite

Accelerator Physics suite
MIT License
1 stars 2 forks source link

Fix makefile #278

Closed anacso17 closed 1 week ago

anacso17 commented 1 month ago

I had problems running Makefile in deploy procedure. This PR replaces spaces with tabs, which fixes the problem.

anacso17 commented 1 month ago

Ok, but I think it is important to understand why this solves the problem.

Hi @fernandohds564 , it seems this bug was introduced in the commit https://github.com/lnls-fac/apsuite/commit/db7913be99f287dcfc60b35a5f637f97dd315212

fernandohds564 commented 1 month ago

Hi @anacso17, it seems Makefile must have tabs to work, right? That's very annoying, considering my code editors are configured to insert spaces instead of tabs.

anacso17 commented 1 month ago

Hi @anacso17, it seems Makefile must have tabs to work, right? That's very annoying, considering my code editors are configured to insert spaces instead of tabs.

Hi @fernandohds564 , yes, Makefiles do not work with spaces...

fernandohds564 commented 1 month ago

@anacso17 , take a look a this:

https://retrocomputing.stackexchange.com/questions/20292/why-does-make-only-accept-tab-indentation

specially these parts:

Why the tab in column 1? Yacc was new, Lex was brand new. I hadn't tried either, so I figured this would be a good excuse to learn. After getting myself snarled up with my first stab at Lex, I just did something simple with the pattern newline-tab. It worked, it stayed. And then a few weeks later I had a user population of about a dozen, most of them friends, and I didn't want to screw up my embedded base. The rest, sadly, is history.

So even though I knew that "tab in column 1" was a bad idea, I didn't want to disrupt my user base.

So instead I wrought havoc on tens of millions.

I have used that example in software engineering lectures.

I doesn't "make" any sense.

what if we start making use of this variable: .RECIPEPREFIX in our Makefiles?

https://www.gnu.org/software/make/manual/html_node/Special-Variables.html

I like the idea of using > to indicate a recipe.

@xresende, what do you think?

xresende commented 1 month ago

my vs code recognizes Makefiles and inserts tabs instead of spaces. I would rather keep the standard use of tabs in makefiles. would vs code insert spaces instead when .RECIPEPREFIX is used ? needs to be checked.

fernandohds564 commented 1 month ago

Well, I don't know if I'm being completely newbie about this, but the fact that of some indentations in a Makefile may indicate completely different things, depending whether it is made of a tab (recipe) or spaces (make directives) is a game changer for me... I didn't know about these two distinct languages in one file before this bug was found. Now that I know, I find it really confusing and difficult to read and understand the code, given that the visualization of both types of indentations are very similar.

If you guys decide it is still worth keeping the standard, Ok, I'll go with it, but I think it would be easier for newbies like myself if the separation between these two languages were more explicit.

I noticed that my vscode also identifies that the majority of the indentations in a Makefile are tabs and also adds tabs for new indentations. But it does that for both types, recipes and make directives, so it is still tricky to identify which one is which. I don't think vscode knows it is a Makefile, I think it just go with the standard of the file. I think the same strategy applies for the number of spaces that constitute an indentation level. If the file uses two spaces, then it also introduces two spaces.

I'll approve this PR, not to hold it longer than what is required. But let me know what you think about this.

xresende commented 1 month ago

i agree. vscode probably infer using tab/space by file inspection. i also find makefile syntax confusing. but using .RECIPEPREFIX requires replacing tabs for space everywhere, right? i dont mind using it, if you find it more useful. no strong opinion about it other than "it has not been an issue for me".

fernandohds564 commented 1 month ago

i agree. vscode probably infer using tab/space by file inspection. i also find makefile syntax confusing. but using .RECIPEPREFIX requires replacing tabs for space everywhere, right? i dont mind using it, if you find it more useful. no strong opinion about it other than "it has not been an issue for me".

It wouldn't require changing tabs by spaces, but I think it would be better if we changed. It would require, though, the addition of the chosen character (>, for instance) at the beginning of every recipe line.