search

sugarlabs / sugar-toolkit-gtk3

Sugar Learning Environment, Activity Toolkit, GTK 3.
GNU Lesser General Public License v2.1
21 stars 80 forks source link

Handle git submodules in bundlebuilder #298

Closed samdroid-apps closed 8 years ago

samdroid-apps commented 8 years ago

Extends the fix of #296

Git submodules are only listed by their root directory in the output of "git ls-files". Therefore, we must handle this case so that the files from the module are included in the install and dist_xo commands.

To reproduce the issue, you could do something like the following:

cd pippy
git clone https://github.com/samdroid-apps/collabwrapper
git add collabwrapper
python setup.py install  # or just osbuild run
godiard commented 8 years ago

Please see my comment in #296

samdroid-apps commented 8 years ago

@godiard said on the other pull request:

I see a potential problem with including submodules. Until now, activities code was always in a single repository, then was easy to young developers clone and start coding. If we allow the use of submodules, or simlinks, will be more complex, and we need document in the activities what is the procedure to get the sources. And this do not solve the issue of change a submodule that need changes in the activity and all stay in a inconsistent state.

Symbolic links are a bad idea as Quozl said on the other patch, apologies for my naivity of suggesting them. They are not handled in this patch.

Git submodules are more complex than straight git as you say. However it solves an important issue for collab wrapper which is "how do we not end up with groupthink 2.0". (groupthink is a collab framework used in pippy and some others). Groupthink seems pretty bad as it has been copied and pasted between activities, whereas it should be a project of it's own. Submodules fix this issue for collab wrapper by allowing activities to not have to copy and paste lots of code to use it. It also allows for collab wrapper to change, but activities can pin the version that they use. Ultimately, collabwrapper should be merged, however this patch helps fill the gap.

This is especially important now that collabwrapper has expanded to 2 files with the collaberative text buffer/editor. This in particular is gonna be in implementation flux as it matures and handles more edge cases.

I believe that cutting down code copy and paste is more important than avoiding submodules.

godiard commented 8 years ago

@samdroid-apps, both approach have good and bad points. I suggest you propose the change and discuses the implications in mailing list. This is not a simple fix, change important assumptions in how the activities are developed.

quozl commented 8 years ago

Discussing and merging this is becoming more urgent, as things are breaking. Pippy-68 released via ASLO has no collabwrapper in the bundle despite https://github.com/walterbender/Pippy/commit/8f184ab637953007b3cab013d1e9ff2247cea6b7 presumably because it was built on a system without #298 applied. Therefore Pippy-68 fails to start.

cc @walterbender.

samdroid-apps commented 8 years ago

This could be the time to discuss and act now, as we are in a feature window!

samdroid-apps commented 8 years ago

Given the lack of objections, this will be part of 0.109.0.1