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

Add a new func_trace builtin #125

Draft
wants to merge 1 commit into
base: add-function-trace
Choose a base branch
from

Conversation

deliciouslytyped
Copy link

In which I cannibalize func_error and func_strip into a new func_trace builtin used as $(trace ...).
This is used similar to tracing function in functional programming languages: they both print (like $(info ...))
and return their argument.

A test run during development looked like:

$ nix-shell -I nixpkgs=channel:nixos-unstable -p gdb '((enableDebugging remake).overrideAttrs (o: { patches = [ ./trace.patch ]; NIX_CFLAGS_COMPILE="-O0"; }))' git \
    --run 'env -i SHELL=bash TERM=screen $(which gdb) $(which remake) -q -ex "b func_call" -ex "directory /remake/src" -ex "dis" -ex "r -X < <(echo -e \"expand \\\$(trace somestring)\\nq\")" -ex "q"'
/nix/store/wvr3sj436yjla36i0fgrwpi6cpbx5rax-gdb-10.2/bin/gdb: warning: Couldn't determine a path for the index cache directory.
Reading symbols from /nix/store/a68rnl52gqg2kfsk0bi9jxz1pbjfqr1s-remake-4.3+dbg-1.5/bin/remake...
Breakpoint 1 at 0x418c24: file src/function.c, line 2428.
Source directories searched: /remake/src:$cdir:$cwd
Starting program: /nix/store/a68rnl52gqg2kfsk0bi9jxz1pbjfqr1s-remake-4.3+dbg-1.5/bin/remake -X < <(echo -e "expand \$(trace somestring)\nq")
Reading makefiles...
hello

Updating makefiles...
Updating goal targets...
 File 'all' does not exist.
Must remake target 'all'.
Successfully remade target file 'all'.
<- (/tmp/tmp.h4PduA6Uid/Makefile:12)
all
remake<0> expand $(trace somestring)
somestring
somestring
remake<1> q
remake: That's all, folks...

[Inferior 1 (process 30082) exited with code 0115]

In which I cannibalize func_error and func_strip into a new func_trace builtin used as `$(trace ...)`.
This is used similar to tracing function in functional programming languages: they both print (like `$(info ...)`)
and return their argument.

A test run during development looked like:
```
$ nix-shell -I nixpkgs=channel:nixos-unstable -p gdb '((enableDebugging remake).overrideAttrs (o: { patches = [ ./trace.patch ]; NIX_CFLAGS_COMPILE="-O0"; }))' git \
    --run 'env -i SHELL=bash TERM=screen $(which gdb) $(which remake) -q -ex "b func_call" -ex "directory /remake/src" -ex "dis" -ex "r -X < <(echo -e \"expand \\\$(trace somestring)\\nq\")" -ex "q"'
/nix/store/wvr3sj436yjla36i0fgrwpi6cpbx5rax-gdb-10.2/bin/gdb: warning: Couldn't determine a path for the index cache directory.
Reading symbols from /nix/store/a68rnl52gqg2kfsk0bi9jxz1pbjfqr1s-remake-4.3+dbg-1.5/bin/remake...
Breakpoint 1 at 0x418c24: file src/function.c, line 2428.
Source directories searched: /remake/src:$cdir:$cwd
Starting program: /nix/store/a68rnl52gqg2kfsk0bi9jxz1pbjfqr1s-remake-4.3+dbg-1.5/bin/remake -X < <(echo -e "expand \$(trace somestring)\nq")
Reading makefiles...
hello

Updating makefiles...
Updating goal targets...
 File 'all' does not exist.
Must remake target 'all'.
Successfully remade target file 'all'.
<- (/tmp/tmp.h4PduA6Uid/Makefile:12)
all
remake<0> expand $(trace somestring)
somestring
somestring
remake<1> q
remake: That's all, folks...

[Inferior 1 (process 30082) exited with code 0115]
```
@rocky rocky changed the base branch from remake-4-3 to add-function-trace August 2, 2021 18:55
@deliciouslytyped
Copy link
Author

Usage example:

$ cat Makefile 
define make-dir
$1/.dum: $(dir $1)
        mkdir $(dir $@)
        touch $(dir $@)/.dum
endef

$(trace $(call make-dir,test))

all:

$ remake -r
test/.dum: ./
        mkdir 
        touch /.dum
remake: *** No rule to make target '
', needed by 'test/.dum'.  Stop.


@rocky
Copy link
Owner

rocky commented Aug 2, 2021

In theory, I think this is great idea and would like to see it pursued. The implementation here I don't see as being complete or that helpful: I have to rewrite my entire source to be able to use this. And if a function calls another function, I have to wrap those as well.

Probably better is a way to have each function (or a set of listed functions) traced showing arguments.

@deliciouslytyped
Copy link
Author

deliciouslytyped commented Aug 2, 2021

If I understood you correctly, yes this is how these kinds of functions are supposed to work, you just wrap them around individual points of interest.

I've now x-linked this to the originating issue #124 .

Another part of the rationale is that it's composable and can be used in "any" situation. This does not necessarily preclude implementing something like what you said, I think they are somewhat orthogonal implementation-wise.

With that said, I'm nowhere near an experienced make user and I'm not able to judge if this generally has the desired semantics, nor am I able to give users advice about using this new primitive in weird make syntax cases.

Also I think some of your message got cut off?

Also, this isn't in any way remake specific, so it could in theory be upstreamed, but I don't really want to figure out how to deal with that right now. It seems easier here.

@rocky
Copy link
Owner

rocky commented Aug 2, 2021

If I understood you correctly, yes this is how these kinds of functions are supposed to work, you just wrap them around individual points of interest.

I suspect this might be doable in GNU Make using the existing set of functions.

Also I think some of your message got cut off?

The fragmentary stuff is removed now.

Also, this isn't in any way remake specific, so it could in theory be upstreamed, but I don't really want to figure out how to deal with that right now. It seems easier here.

Sure, that makes sense.

But still, what you are doing has to make sense and have some value, and I am not convinced right now. So maybe this should hang out until a more convincing use case is at hand. If you use this to debug something you are doing, describe how this helped.

@deliciouslytyped
Copy link
Author

deliciouslytyped commented Aug 2, 2021

Well, I don't see any other way to look at expansions other than doing something like this, and it's nicer than $(call trace,...).
With regards to doing it with a macro, I didn't figure out how to do that properly.

If this isn't enough, I don't really know how to convince you.
I'm using it for exploratory stuff as I learn make and double check it's doing what I hoped.

  • but I just made this patch today, so it hasn't been tested much.

@deliciouslytyped
Copy link
Author

Ok I got some advice on how to maybe reply better.

Having this as a feature increases visibility into (deeply) nested code: it allows you to inspect intermediate values without larger refactorings. The trace function is supposed to transparently pass its argument through as if it wasn't there.

It provides a capability that is otherwise lacking.
Lacking a debugger feature for stepping through macro expansions, I don't see how to get the equivalent of a trace function or macro, with the existing remake functionality - other than writing the same thing.

How do you currently debug macro expansions? I actually don't know.

The actual usage example above is not particularly interesting, because I am not an advanced make user and don't have the skills or use cases to make anything crazy, however that's just more justification for my need to be able to confirm that my code is doing what it should be. So if I want to do that, now I just wrap the expression in $(trace ...) and check that it did what it's supposed to.

I didn't see any make flags for debugging expansions, I looked. I also checked the manual, the two GNU make books (not too carefully though), and the repl help.

It may be possible to do this in a slightly more roundabout manner, or tell people to copy the macro somewhere if there was one, but this is what standard libraries are for, and gmake doesn't really seem to have one as far as I can tell.

Regarding bloat;
This isn't a strong point, but if info, warning, and error exist as functions, I don't see why this one shouldn't besides possibly bloat, which is addressed above. This is a simple implementation and provides a useful inspection feature in a relevant codebase: remake is for debugging makefiles. - even though the code is sufficiently isolated and general to possibly be relevant to upstream. Any of the other implementations for a similar feature would require a lot more work (though they also have somewhat differring capabilities).

This works here and now and it's not crippled/arbitrarily bad design as far as I can tell? It is however limited, as many primitives or basic implementations are.

@deliciouslytyped
Copy link
Author

deliciouslytyped commented Aug 2, 2021

Third attempt. I'm learning;

How do you figure why this doesn't do anything?

all:

define _make-dir
$1/.dum: $(dir $1)
        mkdir $(dir $@)
        touch $(dir $@)/.dum
endef

define make-dir
  $(if $(not-dir $1),$(call _make-dir,$1),)
endef

$(trace $(call make-dir,test/wat/))

$(call make-dir,test/)
$(call make-dir,test/1/)

I was looking at this code because it didn't do anything.
I wanted to develop some directory creation code, and it wasn't working.
I didn't want to factor out random sub-expressions just to see what their value was.
That's a debuggers job. The debugger can't do it.
The next best thing is to be able to inspect things with minimal code modifications, and the obvious (to me) solution is a transparent wrapper for sub-expressions.
So I implemented $(trace ...).
This let me quickly put some wrappers in various sub-expressions[1] in the Makefile without significantly altering (or having to spend mental resources on altering) the structure of the code.

I don't actually expect you to accept the code here, there aren't even tests (I don't know if there even are tests) or documentation included in this patch, but I do believe the idea has merit.

[1] specifically, the problem is that I accidentally used not-dir instead of dir in the if condition.

@deliciouslytyped
Copy link
Author

deliciouslytyped commented Aug 2, 2021

TODO:
1)

all:

define dirdum
  $1/.dum
endef

define _make-dir
$(call dirdum,$1): $(call dirdum,$(dir $1))
        mkdir $(@D)
        touch $(@D)/.dum
endef

define make-dir
#  $(if $(dir $1),$(call _make-dir,$1),)
  $(call _make-dir,$1)
endef

$(trace $(call make-dir,test/))
$(call make-dir,test/1/)

results in:

$ remake -r
#    test//.dum:   test//.dum
        mkdir 
        touch /.dum
    test//.dum:   test//.dum
        mkdir 
        touch /.dum
Makefile:18: *** multiple target patterns.  Stop.

I'm not sure if that comment issue is normal macro behavior.

Trace should have a prefix and line information or something; it's hard or impossible to tell if you're tracing an empty value
3)
Maybe trace shouldn't expand it's arguments?

@rocky rocky marked this pull request as draft August 2, 2021 23:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants