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

Fix bug where dotfiles are found in package root #73 #229

Merged
merged 6 commits into from
Sep 28, 2022
Merged

Fix bug where dotfiles are found in package root #73 #229

merged 6 commits into from
Sep 28, 2022

Conversation

JehhB
Copy link
Contributor

@JehhB JehhB commented Sep 3, 2022

#73

@hwittenborn
Copy link
Member

Hey, thanks for the PR! I'm working on some other stuff at the moment, but I'll try to get this merged by tomorrow or Monday.

@hwittenborn hwittenborn linked an issue Sep 6, 2022 that may be closed by this pull request
@hwittenborn
Copy link
Member

Not sure what's up, but these changes seem to be causing the false positives, I've never personally encountered them before, and it looks like the CI hasn't either.

What environment are you using makedeb in?

@JehhB
Copy link
Contributor Author

JehhB commented Sep 6, 2022

I'm using debian unstable

@knokelmaat
Copy link

I have the same issue, also on debian unstable. Anything we can do on our end to help you debug?

@knokelmaat
Copy link

I modified the original dotfiles.sh to print some directory info:

check_dotfiles() {
	local ret=0

	ls "$pkgdir" 			# Extra info
	echo "$pkgdir"/.* 		# Extra info

	for f in "${pkgdir}/.*"; do

    		echo $f 		# Extra info
		echo ${f##*/} 		# Extra info
		echo . 			# Extra info

		[[ ${f##*/} == . || ${f##*/} == .. ]] && continue
		error "$(gettext "Dotfile found in package root '%s'")" "$f"
		ret=1
	done
	return $ret
}

This gives the following output:

christophe@localhost:~/Downloads/una-bin$ makedeb -si
[#] Making package: una-bin 3.2.0-1 (Tue 06 Sep 2022 10:32:15 AM CEST)
[#] Checking for missing dependencies...
[#] Loading extensions...
[#] Retrieving sources...
  [->] Found una
  [->] Found una-completion
  [->] Found una-updater
  [->] Found una.8.adoc
[#] Validating source files with sha256sums...
    una ... Skipped
    una-completion ... Skipped
    una-updater ... Skipped
    una.8.adoc ... Skipped
[#] Extracting sources...
[#] Removing existing $pkgdir/ directory...
[#] Entering fakeroot environment...
[#] Starting package()...
[#] Checking for packaging issues...
usr
/home/christophe/Downloads/una-bin/pkg/una-bin/.*
/home/christophe/Downloads/una-bin/pkg/una-bin/.*
.*
.
[!] Dotfile found in package root '/home/christophe/Downloads/una-bin/pkg/una-bin/.*

@hwittenborn
Copy link
Member

Sorry for the late reply @JehhB and @knokelmaat, I didn't get anything in my GitHub notifications or I just overlooked it or something :P.

I have the same issue, also on debian unstable. Anything we can do on our end to help you debug?

Not at the moment, I should be able to get a Debian unstable Docker container going shortly to get this all tested, I think a know a pretty good way to get this solved though.

done
return $ret
local ret=0
for f in $(ls -Ad "$pkgdir"/.*); do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should be able to just avoid ls completely by using something like this. It does add another variable into the function, but it stops all this globbing nonsense that's causing issues:

local dotfiles
mapfile -t dotfiles < <(find -mindepth 1 -maxdepth 1 | sed 's|^\./||' | grep '^\.')

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if I replace line 32 with

for f in $(find "$pkgdir" -mindepth 1 -maxdepth 1 -name '.*'); do

It also eliminate the globing issue and also doesn't introduce any new variable. I think it also better to keep the same error message wherein the full path of the dotfile is displayed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there's a dotfile named something like .one two, then that gets printed as two separate files with that approach. It's not the end of the world if that were to happen, but I'd prefer to stick with mapfile just so errors all line up properly.

@hwittenborn
Copy link
Member

Hey I'm gonna go ahead and get this merged, all these MRs have been around for quite a bit and I'm just wanting them closed so they ain't lingering around forever

@hwittenborn hwittenborn merged commit 7c89ae5 into makedeb:alpha Sep 28, 2022
hwittenborn added a commit that referenced this pull request Sep 28, 2022
Co-authored-by: Hunter Wittenborn <hunter@hunterwittenborn.com>
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.

Dotfile found in package root
3 participants