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

Cleanup and update NMake Makefiles. #1673

Merged
merged 1 commit into from
Feb 24, 2024
Merged

Cleanup and update NMake Makefiles. #1673

merged 1 commit into from
Feb 24, 2024

Conversation

mtl1979
Copy link
Collaborator

@mtl1979 mtl1979 commented Feb 19, 2024

  • ARM and ARM64 Makefiles had dependencies for X86 specific headers
  • Some dependencies were mentioned more than once for same object file
  • Use only $(TOP) as NMake doesn't change directories while building, $(SRCDIR) was redundant
  • Sort the dependencies and separate dependencies for test binaries.

Copy link

codecov bot commented Feb 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.04%. Comparing base (9953f12) to head (a761ae0).

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #1673   +/-   ##
========================================
  Coverage    83.03%   83.04%           
========================================
  Files          134      134           
  Lines        10336    10336           
  Branches      2813     2813           
========================================
+ Hits          8583     8584    +1     
  Misses        1056     1056           
+ Partials       697      696    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@nmoinvaz
Copy link
Member

Did you make a script to do this or did you do it manually?

@mtl1979
Copy link
Collaborator Author

mtl1979 commented Feb 19, 2024

Did you make a script to do this or did you do it manually?

I made an executable, I will make another PR/commit when I have integrated it to CI...

@nmoinvaz
Copy link
Member

I see. Or, if you want you can just make a GitHub gist and paste the code there.

@mtl1979
Copy link
Collaborator Author

mtl1979 commented Feb 19, 2024

I see. Or, if you want you can just make a GitHub gist and paste the code there.

https://gist.github.com/mtl1979/83d0691c0c2a03154ffbea473a85a6c3

@nmoinvaz nmoinvaz added cleanup Improving maintainability or removing code. Build Env labels Feb 20, 2024
@Dead2 Dead2 added the Rebase needed Please do a 'git rebase develop yourbranch' label Feb 22, 2024
@Dead2
Copy link
Member

Dead2 commented Feb 22, 2024

@mtl1979 This needs a rebase

@phprus
Copy link
Contributor

phprus commented Feb 23, 2024

@mtl1979
Please provide a simple example of using depcheck.cpp to check NMake dependencies.

@mtl1979
Copy link
Collaborator Author

mtl1979 commented Feb 23, 2024

@mtl1979 Please provide a simple example of using depcheck.cpp to check NMake dependencies.

I run it from root of the repository using for example depcheck.exe win32\Makefile.msc . ...

Running the check on Makefiles one by one is easier for a human than running nmake -f win32\Makefile.msc depcheck which will check all three Makefiles in batch, as it will be harder to track which one of the three Makefiles has missing or extra dependencies... When it detects an error, the log should be read from bottom to top to see which line has the error. It will use prefix File: when it starts reading a new line. It will currently only check direct dependencies. I've whitelisted some known indirect dependencies until there is enough to warrant checking all the dependencies, not just the source file.

@mtl1979
Copy link
Collaborator Author

mtl1979 commented Feb 23, 2024

depcheck.exe will now print current Makefile and object file when it encounters either missing or unnecessary dependency.

* Add depcheck.exe to validate NMake Makefiles
@Dead2 Dead2 merged commit 1d08728 into zlib-ng:develop Feb 24, 2024
140 checks passed
@Dead2 Dead2 mentioned this pull request May 30, 2024
@Dead2 Dead2 mentioned this pull request Jun 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build Env cleanup Improving maintainability or removing code. Rebase needed Please do a 'git rebase develop yourbranch'
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants