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

Intialise dep_fd earlier to write negative dependencies correctly #36

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

lions-tech
Copy link

During the execution of find_dofile we need to write to the dependency file; at least if a dofile is not found in the current directory. This is what redo_ifcreate called by check_dofile does.

However, the dependency file descriptor dep_fd is initiated after find_dofile is called.

As a result, the dep_fd for the first target is -1, so there are no negative dependencies to dofiles. Moreover, the negative dependencies are written to the dep_fd of the previous target. This behaviour is especially problematic, when writing the negative dependencies for the first target of a new subdirectory.

If you wish to reproduce this behaviour, you may use my dummy project: https://github.com/lions-tech/redo-c-testproj

@leahneukirchen
Copy link
Owner

Yes, but I think this needs more cleanup code now.

@lions-tech
Copy link
Author

This might be due to my limited C knowledge, but I don't see anything that needs to be cleaned up.

I tested the implementation on my dummy project and it seemed to work fine. The temporary files were all renamed/removed correctly.

The find_dofile function doesn't seem to do anything else than writing to the dep_fd and creating the lock file and the target file after the dependency should'nt be a problem as well.

@leahneukirchen
Copy link
Owner

There are two exits and one return now that can leave around the tempfile.

@lions-tech
Copy link
Author

I see what you mean. How about a generic tempfile cleanup function? I noticed tempfiles are also left over, if errors occured in the dofile.

@lions-tech lions-tech force-pushed the fix-writing-to-wrong-depfile branch from 200718a to 9f0f8dd Compare September 22, 2023 09:00
@lions-tech lions-tech marked this pull request as draft October 4, 2023 05:37
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.

2 participants