juju / charm-helpers

Apache License 2.0
18 stars 127 forks source link

`git add` for untracked files in charm_helpers_sync.py #383

Open dshcherb opened 5 years ago

dshcherb commented 5 years ago

Just encountered a situation where the current make sync behavior with using charm_helpers_sync.py is lacking.

charm_helpers_sync.py currently does not do git add after it downloads charm-helpers. https://github.com/juju/charm-helpers/blob/master/tools/charm_helpers_sync/charm_helpers_sync.py

A natural thing to do is git add -u && git commit -m '...', however, for files in charm-helpers not previously tracked by the particular charm repo itself, this will not be enough.

A recent sync left me with the following files untracked:

git status
On branch 1842353-add-port-forwarding
Untracked files:
  (use "git add <file>..." to include in what will be committed)

    hooks/charmhelpers/contrib/openstack/policyd.py
    hooks/charmhelpers/fetch/ubuntu_apt_pkg.py

nothing added to commit but untracked files present (use "git add" to track)

Which then resulted in a functional test failure due to a missing module:

2019-10-06 17:43:08 DEBUG install   File "/var/lib/juju/agents/unit-neutron-openvswitch-0/charm/hooks/charmhelpers/fetch/ubuntu.py", line 32, in <module>
2019-10-06 17:43:08 DEBUG install     from charmhelpers.fetch import ubuntu_apt_pkg
2019-10-06 17:43:08 DEBUG install ImportError: cannot import name 'ubuntu_apt_pkg'
2019-10-06 17:43:08 ERROR juju.worker.uniter.operation runhook.go:132 hook "install" failed: exit status 1

The module is indeed missing: https://review.opendev.org/gitweb?p=openstack/charm-neutron-openvswitch.git;a=tree;f=hooks/charmhelpers/fetch;hb=c6256fa069fae0f210efc2be2a8401d315ae0e8a

It would be useful to do git add after a sync is made automatically in charm_helpers_sync.py.

ajkavanagh commented 5 years ago

I'm not sure I understand your workflow here. With a 'legacy' charm, normally make sync is run manually to bring the charmhelpers files into the charm. Then one manually checks that everything looks okay and adds/updates any new charmhelper files.

However, I can see a perspective where the make sync ought to do a git add <charmhelpers-dir> out of courtesy. Is that what you mean?

dshcherb commented 5 years ago

@ajkavanagh

Is that what you mean?

Yes.

ajkavanagh commented 5 years ago

So, it would probably be reasonably complicated. In theory, the charm may be managed under a different vcs than git, so the code would need to check that before doing anything.

I wonder if the simplest approach isn't to add it to the charm's Makefile instead? i.e make sync && git add <whatever-the-charmhelpers-dir-is-for-that-charm>. It's not done centrally, but it would mean that the make sync code wouldn't have to be vcs-type aware. Thoughts?

dshcherb commented 5 years ago

@ajkavanagh

It's not done centrally

I think that's the key point. I wonder if just checking for .git presence and then attempting git add is reasonable.