Imports do not represent empty directories

Bug #1687057 reported by Nish Aravamudan
10
This bug affects 1 person
Affects Status Importance Assigned to Milestone
git-ubuntu
Fix Released
Undecided
Robie Basak
dgit (Debian)
New
Unknown

Bug Description

At least src:software-properties has at one time had empty directories
in the source package, e.g. in 0.96.20.5 in xenial on Ubuntu, and git
(and thus the Ubuntu git importer) fail to
properly represent the srcpkg's contents in the import.

Specifically, tests/aptroot/etc/apt/apt.conf.d/ is missing from the import.

Given that git does not represent empty directories, I'm not sure what
we should do here.

List of known affected packages:

apache2
apparmor
debootstrap
exim4
gettext
mongodb
nagios-nrpe
ssl-cert
supermin
qemu
nmap

[Workaround]

Use the --no-verify to ignore the error.

Related branches

Revision history for this message
Nish Aravamudan (nacc) wrote :

Affects src:apparmor, src:gettext, src:ssl-cert, src:supermin,

Changed in usd-importer:
assignee: nobody → Robie Basak (racb)
status: New → Confirmed
Revision history for this message
Robie Basak (racb) wrote :

Also affects src:exim4.

description: updated
description: updated
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

I haven't looked at the others yet, but for the exim4 case that is a dir called "Local".
BTW this easily finds offenders:
 $ find . -depth -empty

What I find interesting is that they maintain it in git at https://anonscm.debian.org/git/pkg-exim4/exim4.git

There the clue for them is that they only maintain the Debian dir itself which is free of issues. The "Local" dir in this case is from exim4 tarball.

It would be a major change in tree structure if we would do so as well and keep content outside of debian/ as tarball only. But since we already restore tarballs as needed we could use that.

OTOH we would also loose a lot of ability like easily comparing old and new content between package versions. And also the permanent "dirtiness" of the Tree when unpacked would be something I'd not like.

The suggestions of searches lead to files like .gitkeep, and "usd build" could maybe build and restore those as needed.

Looking forward to discuss later today ....

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

well the empty also spots empty files :-/, but one can add "-type d" to fix that.

I wanted to comlete the circle for exim and since it is developed upstream in git as well there is no empty Dir there either.
When upstream creates their tarballs from git the dir is created.

Nish Aravamudan (nacc)
description: updated
Robie Basak (racb)
description: updated
Nish Aravamudan (nacc)
description: updated
Changed in dgit (Debian):
status: Unknown → New
Robie Basak (racb)
description: updated
Nish Aravamudan (nacc)
Changed in usd-importer:
status: Confirmed → Fix Released
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

also affects nmap

description: updated
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

also affects apache2 (xenial at least)

WARNING: empty directories exist but are not tracked by git:

docs/manual/style/lang
docs/manual/style/xsl/util

These will silently disappear on commit, causing extraneous
unintended changes. See: LP: #1687057.

description: updated
Bryce Harrington (bryce)
description: updated
Revision history for this message
Rafael David Tinoco (rafaeldtinoco) wrote :

Also affects samba ?

(c)inaddy@ctdbdisco:~/work/sources/ubuntu/samba$ git cherry-pick --continue
ERROR: Empty directories exist and will disappear on commit, causing
extraneous unintended changes:

source4/ldap_server/devdocs

See LP: #1687057.
Use "git commit --no-verify ..." to ignore this problem.

pre-commit hook failed.

Revision history for this message
Bryce Harrington (bryce) wrote :

There may also be old branches affected as well

$ git checkout remotes/pkg/ubuntu/xenial-security -b xenial-security
Branch 'xenial-security' set up to track remote branch 'ubuntu/xenial-security' from 'pkg' by rebasing.
Switched to a new branch 'xenial-security'

WARNING: empty directories exist but are not tracked by git:

crypto/des/t
include

These will silently disappear on commit, causing extraneous
unintended changes. See: LP: #1687057.

Revision history for this message
Bryce Harrington (bryce) wrote :

Christian's right that 99% of suggestions are to add a .gitkeep or similar, but found a couple other kludgy workaround ideas:

1. Add the empty dir as a submodule
    https://stackoverflow.com/a/58543445

    find . -type d -empty -delete -exec git submodule add -f https://gitlab.com/empty-repo/empty.git \{\} \;

2. Add a broken symlink in the empty dir
    https://stackoverflow.com/a/37597601/1004236

    mkdir empty
    ln -s .this.directory empty/.keep
    git add -f empty/.keep
    git commit -m synthetic-dir

Not sure what side effects or ramifications these would have.

For a proper solution, sounds like git upstream might accept a well-coded fix, but that's probably too out of scope for us for now.

Revision history for this message
Paride Legovini (paride) wrote :

The official git FAQ [1] suggest using an en .gitignore file as a placeholder for empty directories. This is also what git-svn does, see the --placeholder-filename option in git-svn(1).

Paride

[1] https://git.wiki.kernel.org/index.php/GitFaq#Can_I_add_empty_directories.3F

Revision history for this message
Paride Legovini (paride) wrote :

typo fix: an *empty* .gitignore file

Revision history for this message
Robie Basak (racb) wrote : Re: [Bug 1687057] Re: git cannot represent empty directories by default

On Mon, Jun 29, 2020 at 12:37:33PM -0000, Paride Legovini wrote:
> The official git FAQ [1] suggest using an en .gitignore file as a
> placeholder for empty directories. This is also what git-svn does, see
> the --placeholder-filename option in git-svn(1).

Unfortunately that's not an option for git-ubuntu since we can't change
the source that we're importing.

Or if we do, then we have to do it in a reversible way, such as how we
do the .git escaping (if .git is present then it gets imported into git
as ..git).

Otherwise we end up in a situation where a build based on git-ubuntu
sources would fail to build, or result in a different build, from a
build from the source package that git-ubuntu imported.

Revision history for this message
Paride Legovini (paride) wrote : Re: git cannot represent empty directories by default

As I hit this with apache2 I tried to figure out a solution.

If we use an empty file as placeholder in empty directories then I think the build won't fail, as the diff with the orig source tree would still be empty. There's also a list of file patterns that dpkg-source ignores when checking the diff (see `dpkg-source --help`):

'(?:^|/).*~$|(?:^|/)\.#.*$|(?:^|/)\..*\.sw.$|(?:^|/),,.*(?:$|/.*$)|(?:^|/)(?:DEADJOE|\.arch-inventory|\.(?:bzr|cvs|hg|git|mtn-)ignore)$|(?:^|/)(?:CVS|RCS|\.deps|\{arch\}|\.arch-ids|\.svn|\.hg(?:tags|sigs)?|_darcs|\.git(?:attributes|modules|review)?|\.mailmap|\.shelf|_MTN|\.be|\.bzr(?:\.backup|tags)?)(?:$|/.*$)'

so an empty .git-ubuntu-empty-directory-placeholder~ file should work fine.

This file is ignored by dpkg-source (as it matches the first regex and also because it's empty), but it's *also* auto-removed by dh_clean, which runs before the actual build. (See dh_clean(1), where it's documented that *~ files are removed). This should ensure we get identical build results.

Bryce Harrington (bryce)
Changed in usd-importer:
status: Fix Released → Triaged
Revision history for this message
Robie Basak (racb) wrote :

No, this bug represented the importer's output. That output now does represent empty directories correctly, so this bug is resolved.

There are still UX issues at the client end, but we should track that in a separate bug if we aren't already.

Changed in usd-importer:
status: Triaged → Fix Released
Revision history for this message
Bryce Harrington (bryce) wrote :

well that's annoyingly frustrating.

apache2 is really hard to maintain using git ubuntu as is, and paride's suggestion seems worth at least considering.

Revision history for this message
Bryce Harrington (bryce) wrote :

Ftr, this is error I had hit, and the workaround I used:

03/04/2021 06:26:28 - ERROR:Command exited 1: git commit -m merge-changelogs debian/changelog
03/04/2021 06:26:28 - ERROR:stdout:
03/04/2021 06:26:28 - ERROR:stderr: /snap/git-ubuntu/current/bin/bash: warning: setlocale: LC_ALL: cannot change locale\
 (en_US.UTF-8)
  ERROR: Empty directories exist and will disappear on commit, causing
  extraneous unintended changes:

  /snap/git-ubuntu/current/bin/bash: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8)
  docs/manual/style/lang
  docs/manual/style/xsl/util

  See LP: #1687057.
  Use "git commit --no-verify ..." to ignore this problem.

  pre-commit hook failed.

Traceback (most recent call last):
  File "/snap/git-ubuntu/474/bin/git-ubuntu", line 11, in <module>
    load_entry_point('gitubuntu==1.0', 'console_scripts', 'git-ubuntu')()
  File "/snap/git-ubuntu/474/lib/python3.6/site-packages/gitubuntu/__main__.py", line 279, in main
    sys.exit(args.func(args))
  File "/snap/git-ubuntu/474/lib/python3.6/site-packages/gitubuntu/merge.py", line 108, in cli_main
    subcommand=args.subsubcommand,
  File "/snap/git-ubuntu/474/lib/python3.6/site-packages/gitubuntu/merge.py", line 482, in main
    force,
  File "/snap/git-ubuntu/474/lib/python3.6/site-packages/gitubuntu/merge.py", line 335, in do_finish
    do_merge_changelogs(repo, commitish, merge_base_commit_hash, onto)
  File "/snap/git-ubuntu/474/lib/python3.6/site-packages/gitubuntu/merge.py", line 206, in do_merge_changelogs
    repo.git_run(['commit', '-m', 'merge-changelogs', 'debian/changelog'])
  File "/snap/git-ubuntu/474/lib/python3.6/site-packages/gitubuntu/git_repository.py", line 1813, in git_run
    **kwargs,
  File "/snap/git-ubuntu/474/lib/python3.6/site-packages/gitubuntu/git_repository.py", line 274, in git_run
    return run(['git'] + list(args), env=env, **kwargs)
  File "/snap/git-ubuntu/474/lib/python3.6/site-packages/gitubuntu/run.py", line 87, in run
    raise e
  File "/snap/git-ubuntu/474/lib/python3.6/site-packages/gitubuntu/run.py", line 66, in run
    stdout=stdout, stderr=stderr, stdin=stdin)
  File "/snap/git-ubuntu/474/usr/lib/python3.6/subprocess.py", line 418, in run
    output=stdout, stderr=stderr)
subprocess.CalledProcessError: Command '['git', 'commit', '-m', 'merge-changelogs', 'debian/changelog']' returned non-z\
ero exit status 1.

$ git commit -m merge-changelogs --no-verify debian/changelog
[detached HEAD 4180b2f0f] merge-changelogs
 1 file changed, 1670 insertions(+), 2 deletions(-)
$ commit_message=$(for rev in $(git rev-list --reverse 'new/debian..HEAD^'); do git log --pretty=%B -n 1 $rev | sed '/^\
[[:space:]]*$/d'; done)
$ awk -v r="$commit_message" '{gsub(/* PLACEHOLDER/,r)}1' debian/changelog > debian/changelog.tmp
$ mv debian/changelog.tmp debian/changelog
$ git commit --no-verify -m reconstruct-changelog debian/changelog
$ update-maintainer
$ git commit --no-verify -m update-maintainer debian/control

But sounds like there is a better workaround coming soon.

Revision history for this message
Robie Basak (racb) wrote :

Sorry, I didn't intend to dismiss the issue nor shut down any discussion.

Since this bug's status has already been used to some track work on this which has been completed, I just didn't want to use it to track further work. Otherwise we end up with a zombie bug that can never be closed as long as workarounds exist because the workarounds are by definition never perfect. I'd prefer to have separate bugs for separate proposals and work so that progress can effectively be tracked.

To this end I've opened bug 1917877 to keep the UX issue open.

summary: - git cannot represent empty directories by default
+ git-ubuntu imports do not represent empty directories by default
summary: - git-ubuntu imports do not represent empty directories by default
+ git-ubuntu imports do not represent empty directories
summary: - git-ubuntu imports do not represent empty directories
+ Imports do not represent empty directories
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.