orangeduck / mpc

A Parser Combinator library for C
Other
2.69k stars 294 forks source link

dependency errors issue #152

Open Meiye-lj opened 1 year ago

Meiye-lj commented 1 year ago

Hi,

We recently conducted a study to detect build dependency errors, focusing on missing dependencies and redundant dependencies. A missing dependency (MD) is a dependency that is not declared in the build script and a redundant dependency(RD) is a dependency that is declared in the build script that is not actually used. We have detected the following dependency errors in your public projects. Could you please help us to check these dependency errors?

MS 0['/home/lv/WorkSpace/vmake_experiment/mpc-master/build/mpc.o---libmpc.a']

RD 0['build---examples/tree_traversal'] 1['build---examples/line_reader'] 2['build---examples/smallc'] 3['build---examples/foobar'] 4['build---examples/maths'] 5['build---examples/lispy'] 6['build---examples/doge'] 7['build---test-file'] 8['build---libmpc.so'] 9['mpc.c---libmpc.a'] 10['mpc.h---libmpc.a']

HalosGhost commented 1 year ago

If MS is a typo for MD, no, mpc.o and libmpc.a are built in this repository itself, not external dependencies.

For the redundant dependencies, these are all also built as a part of building this project. Are you detecting these as being redundant dependencies in some of the targets in the Makefile? If so, which targets have these as redundant dependencies?

Meiye-lj commented 1 year ago

Hi,Thanks for your feedback!The data format is in execl is dependency -- target. For miss dependencies, we observed that when mpc. o changes, incremental builds are performed, and libmpc. a is not updated. In the actual process of building, mpc. o is the dependency of libmpc. a, but this is not stated in the build script.For the redundant dependencies,the mpc.c is a redundant dependency of libmpc.a.


----- 原始邮件 ----- 发件人:Sam Stuewe @.> 收件人:orangeduck/mpc @.> 抄送人:Meiye-lj @.>, Author @.> 主题:Re: [orangeduck/mpc] dependency errors issue (Issue #152) 日期:2022年12月09日 14点00分

If MS is a typo for MD, no, mpc.o and libmpc.a are built in this repository itself, not external dependencies. For the redundant dependencies, these are all also built as a part of building this project. Are you detecting these as being redundant dependencies in some of the targets in the Makefile? If so, which targets have these as redundant dependencies?

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: @.***>

HalosGhost commented 1 year ago

Ahh, I see; so you're saying these are dependencies in the Makefile sense (i.e., that some rules depend on files without making that explicit). That's very possible. I'll try to poke around with that in the next few days if I have some free time (both to evaluate and potentially fix).

Meiye-lj commented 1 year ago

Thank you and I look forward to your follow-up response. ----- 原始邮件 ----- 发件人:Sam Stuewe @.> 收件人:orangeduck/mpc @.> 抄送人:Meiye-lj @.>, Author @.> 主题:Re: [orangeduck/mpc] dependency errors issue (Issue #152) 日期:2023年01月04日 01点54分

Ahh, I see; so you're saying these are dependencies in the Makefile sense (i.e., that some rules depend on files without making that explicit). That's very possible. I'll try to poke around with that in the next few days if I have some free time (both to evaluate and potentially fix).

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: @.***>

HalosGhost commented 1 year ago

Okay, now looking into this, I actually don't agree with this analysis.

The dependency on the build directory is (admittedly) a little funny, but it works to ensure that no matter which target you choose to build first, the build directory will be created as-needed. And, having libmpc.a (and libmpc.so) depend on mpc.c and mpc.h, it ensure they'll be rebuilt if you modify either source file. This could also be accomplished by building object-files separately first, but that isn't how it was set up. Finally, also a little funny, libmpc.a actually makes mpc.o unconditionally first. So, having mpc.o as a separate dependency actually would be redundant.

Maybe a good cleanup would be to separate building object files, and then building binaries as a second step (and maybe removing the $(DIST) dependency in favor of making the first line of those rules being $(MKDIR) $(@D)). But, as far as I can tell, there aren't actually any missing or specifically redundant dependencies.

Meiye-lj commented 1 year ago

Thank you for your first. However, in our analysis we have tried to modify the modification time of mpc.o to trigger an incremental build. At this point libmpc.a should also be re-built, but since it is not declared in the Makefile (missing dependency). libmpc.a is not be re-built. ----- 原始邮件 ----- 发件人:Sam Stuewe @.> 收件人:orangeduck/mpc @.> 抄送人:Meiye-lj @.>, Author @.> 主题:Re: [orangeduck/mpc] dependency errors issue (Issue #152) 日期:2023年01月07日 09点25分

Okay, now looking into this, I actually don't agree with this analysis. The dependency on the build directory is (admittedly) a little funny, but it works to ensure that no matter which target you choose to build first, the build directory will be created as-needed. And, having libmpc.a (and libmpc.so) depend on mpc.c and mpc.h, it ensure they'll be rebuilt if you modify either source file. This could also be accomplished by building object-files separately first, but that isn't how it was set up. Finally, also a little funny, libmpc.a actually makes mpc.o unconditionally first. So, having mpc.o as a separate dependency actually would be redundant. Maybe a good cleanup would be to separate building object files, and then building binaries as a second step (and maybe removing the $(DIST) dependency in favor of making the first line of those rules being $(MKDIR) @.***)). But, as far as I can tell, there aren't actually any missing or specifically redundant dependencies.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: @.***>

HalosGhost commented 1 year ago

This is true, but as I mentioned, that more or less wouldn't matter (because modifying mpc.c or mpc.h would trigger the rebuild). When would you modify mpc.o but not mpc.c or mpc.h in a real-world example?

Meiye-lj commented 1 year ago

This will not be a problem in your Makefile, but please consider the following case.If the structure in the Makefile is "libmpc.a": [anything, ...] (no "mpc.c", "mpc.h", and "mpc.o" are missing), "mpc.o": ["mpc.c", "mpc.h"]. Then even if "mpc.c" or "mpc.h" is changed, the "libmpc.a" will not be re-built correctly.This structure is common in real-world projects. So can you now accept our detection of missing dependencies?


----- 原始邮件 ----- 发件人:Sam Stuewe @.> 收件人:orangeduck/mpc @.> 抄送人:Meiye-lj @.>, Author @.> 主题:Re: [orangeduck/mpc] dependency errors issue (Issue #152) 日期:2023年01月07日 10点48分

This is true, but as I mentioned, that more or less wouldn't matter (because modifying mpc.c or mpc.h would trigger the rebuild). When would you modify mpc.o but not mpc.c or mpc.h in a real-world example?

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: @.***>

HalosGhost commented 1 year ago

Sure, in a makefile where a target doesn't list dependencies (even indirectly), it might lead to the target not being rebuilt appropriately.