Skip to content

Add makedepend.file target in the generated Makefile #23402

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

Open
wants to merge 2 commits into
base: blead
Choose a base branch
from

Conversation

dlaugt
Copy link
Contributor

@dlaugt dlaugt commented Jul 4, 2025

Add makedepend_file.SH in the variable SH of Makefile.SH.

This is for adding the following target in the generated Makefile to recreate makedepend.file when the script makedepend_file.SH has been changed:

makedepend.file: makedepend_file.SH config.sh
    $(SHELL) makedepend_file.SH

The generated file has been renamed from makedepend_file to makedepend.file to be in line with SH_to_target() of Makefile.SH. SH_to_target() replaces in the script name the underscore by dot.

  • This set of changes does not require a perldelta entry.

@jkeenan jkeenan requested review from Corion and nwc10 July 4, 2025 14:36
@richardleach
Copy link
Contributor

The generated file has been renamed from makedepend_file to makedepend.file to be in line with SH_to_target() of Makefile.SH. SH_to_target() replaces in the script name the underscore by dot.

Please could you explain what making this change accomplishes?

Whether the resulting file is named makedepend_file or makedepend.file doesn't seem to make any difference.

@dlaugt
Copy link
Contributor Author

dlaugt commented Jul 4, 2025

Please could you explain what making this change accomplishes?
Whether the resulting file is named makedepend_file or makedepend.file doesn't seem to make any difference.

Exactly, makedepend_file or makedepend.file doesn't make any difference. However, I've renamed the generated file because SH_to_target() with makedepend_file.SH generates the target makedepend.file instead of makedepend_file as follow:

makedepend.file: makedepend_file.SH config.sh
    $(SHELL) makedepend_file.SH

Here is the code of SH_to_target() where there is a sed of _ by .:

SH_to_target() {
    echo $@ | sed -e s/\\\.SH//g -e s/_/./g
}

An example is the target config.h that is already generated by SH_to_target() with config_h.SH:

config.h: config_h.SH config.sh
    $(SHELL) config_h.SH

@richardleach
Copy link
Contributor

However, I've renamed the generated file because SH_to_target() with makedepend_file.SH generates the target makedepend.file instead of makedepend_file

Yes, but why?

config.h is a C header file, so generating this filename follows the normal naming convention.

makedepend_file is a shell script and calling it makedepend.file instead doesn't follow the same sort of naming convention and arguably makes it less obvious that it's an executable file. It might make more sense to make the source file makedependfile_sh.SH to create makedependfile.sh, but again, what's the actual benefit in doing so?

@dlaugt
Copy link
Contributor Author

dlaugt commented Jul 4, 2025

Yes, but why?

I wanted to make as few changes as possible without changing the name of makedepend_file.SH committed in git. However, your observation is pertinent. Perhaps, we can rename the script makedepend_file.SH to makedependfile.SH to generate the script makedependfile which could have similar naming convention as the other generated scripts: cflags, makedepend, myconfig, runtests.

Anyway, the main goal of this pull request is not about renaming. It's about recreating automatically the generated script makedepend.file when the script makedepend_file.SH has been changed by the user. Like what we have for the other *.SH.

@richardleach
Copy link
Contributor

Anyway, the main goal of this pull request is not about renaming. It's about recreating automatically the generated script makedepend.file when the script makedepend_file.SH has been changed by the user. Like what we have for the other *.SH.

Apologies if the answer is obvious, but why not add this target:

makedepend_file: makedepend_file.SH config.sh
    $(SHELL) makedepend_file.SH

@dlaugt
Copy link
Contributor Author

dlaugt commented Jul 4, 2025

This is also another way to do it with even less changes. However, I prefer to use the variable SH which is designed specifically for this purpose (eg, generating automically the files when *.SH has been changed by the user).

@Leont
Copy link
Contributor

Leont commented Jul 4, 2025

Perhaps, we can rename the script makedepend_file.SH to makedependfile.SH to generate the script makedependfile which could have similar naming convention as the other generated scripts: cflags, makedepend, myconfig, runtests.

Yeah I think that makes more sense than renaming it to makedepend.file.

@dlaugt
Copy link
Contributor Author

dlaugt commented Jul 5, 2025

Yeah I think that makes more sense than renaming it to makedepend.file.

Ok, I have added a commit for this renaming.

@jkeenan
Copy link
Contributor

jkeenan commented Jul 5, 2025

I ran sh ./Configure -des -Dusedevel && make test in blead and then after applying this p.r.

Before

-rwxrwxr-x 1 jkeenan jkeenan 4653 Jun  5 18:09 ./makedepend.SH
-rw-rw-r-- 1 jkeenan jkeenan 6499 Jul  4 13:56 ./makedepend_file.SH
-rwxrwxr-x 1 jkeenan jkeenan 3888 Jul  5 18:40 ./makedepend
-rwxrwxr-x 1 jkeenan jkeenan 5704 Jul  5 18:40 ./makedepend_file

After

-rwxrwxr-x 1 jkeenan jkeenan 4653 Jun  5 18:09 ./makedepend.SH
-rw-rw-r-- 1 jkeenan jkeenan 6492 Jul  5 18:42 ./makedependfile.SH
-rwxrwxr-x 1 jkeenan jkeenan 3888 Jul  5 18:43 ./makedepend
-rwxrwxr-x 1 jkeenan jkeenan 5703 Jul  5 18:43 ./makedependfile

Am I correct in thinking that the net effect of this p.r. is:

  • to change the name of source code file makedepend_file.SH to makedependfile.SH; and

  • to change the name of generated file makedepend_file to makedependfile?

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.

4 participants