search

ungoogled-software / ungoogled-chromium

Google Chromium, sans integration with Google
BSD 3-Clause "New" or "Revised" License
20.8k stars 844 forks source link

when pruning files log missing file paths together with the error message #3071

Closed clickot closed 3 weeks ago

clickot commented 3 weeks ago

in case of missing files to prune the prune_binary.py should log their paths together with the error message to ease fixing this. Until now these paths were only logged when debug level is enabled.

PF4Public commented 3 weeks ago

;)

https://github.com/ungoogled-software/ungoogled-chromium/pull/2954#issuecomment-2246230907 https://github.com/ungoogled-software/ungoogled-chromium/pull/2954#issuecomment-2246666744

clickot commented 3 weeks ago

i don't understand what cirrus is complaining about the code formatting, original and reformatted code looks equal to me... @PF4Public i'll double check if this PR is really needed

Ahrotahn commented 3 weeks ago

@PF4Public My main concern at the time was there were 76 files that would be listed.

When there are a lot files that will be listed you can end up in a situation where that list is longer than your terminal buffer, which pushed off the original error and leaves you with less helpful information. Maybe we could instead list the first half-dozen files with a following message saying something like "plus X more files" if there's more than that.

@clickot There's some whitespace at the end of the first line. I couldn't see the difference at first either.

PF4Public commented 3 weeks ago

Maybe we could instead list the first half-dozen files with a following message saying something like "plus X more files" if there's more than that.

That is a good idea! @clickot Would you be able to add this condition?

clickot commented 3 weeks ago

done as requested :) btw: after @Ahrotahn s question in #3069 and @PF4Public hint I noticed that the node_modules.tar.gz file exists after runing the clone script on it's own. so i restarted my build from scratch without removing the line from the pruning list and things went fine. So the reason for this PR has disappeared in a way... but nonetheless it might be useful in future cases :-)

PF4Public commented 3 weeks ago

Would it then make sense to keep listing all of the files with debug flag?

but nonetheless it might be useful in future cases :-)

absolutely! the original idea was even improved :)

clickot commented 3 weeks ago

Would it then make sense to keep listing all of the files with debug flag?

good idea, i'll add that line

clickot commented 3 weeks ago

@PF4Public @Ahrotahn since i have no write access, would one of you be so kind and merge this?

PF4Public commented 3 weeks ago

I guess we're both implicitly waiting for @networkException's comments or silent approval ;)