sproutcore / build-tools

SproutCore Build Tools
12 stars 7 forks source link

Differences in sorting with Abbot #9

Closed mauritslamers closed 10 years ago

mauritslamers commented 10 years ago

In the pursuit of a possible cause for issue #7, I noticed that the sorting technique in the new BT has a few differences with the sorting as done by Abbot. In https://gist.github.com/mauritslamers/11040893 there are two index.html files, one generated by the new BT, the other by abbot, where in both the combining has been turned off to see the load order.

Few things to notice in the comparison:

In DataStore the order is almost entirely different, a few small changes in statechart. The order inside an application is also different. Are there additional rules not observed yet?

mauritslamers commented 10 years ago

Part of the problem seems to have been that for some reason I didn't also sort the filenames alphabetically first. I corrected it somewhere already, but for some reason not on the sass_slicing branch. Sadly it doesn't fix issue #7.

mauritslamers commented 10 years ago

The problem of issue #7 turned out to be caused by an unexpected side effect of the sorting differences between the new BT and Abbot.

Because the file name core_foundation/panes/main.js is "main.js", it is sorted as last file in the framework.

As far as I have understood, the main.js rule was only there for apps to have the apps main.js sorted as last, which in case of an app is perfectly understandable. Until now the sorting of main.js was implemented as [framework_path]/main.js. This strict interpretation of the main.js sorting caused issue #7.

So a small discussion seems to be required here: is the sorting of core_foundation/panes/main.js accidental or intended? My proposal would be to reserve this behaviour to [framework_path]/main.js. The "loose" sorting behaviour would cause all kinds of unexpected side effects in the case someone happens to have a file controllers/main.js for a mainController, and unexpectedly have the file as last file in the sorting.

dcporter commented 10 years ago

Agreed that [app_folder]/subfolder/main.js should not get caught up in the [app_folder]/main.js sorting rule. What's the proximate cause of the infinite loop, is it something we can fix in master so that things build correctly under both rulesets?

mauritslamers commented 10 years ago

It turns out that adding sc_require('panes/pane_statechart') to core_foundation/panes/main.js will do the trick. Cherry-picking this commit should be sufficient: https://github.com/sproutcore/sproutcore/commit/012f90e50a5f0b3c5c2040927f1bbb31650948cd

dcporter commented 10 years ago

Nice! Running Travis on this now; will merge into master as soon as it passes.

dcporter commented 10 years ago

Done! Closing this out on the assumption that we've nailed it.

mauritslamers commented 10 years ago

There still are sorting differences with Abbot. In the TestRunner app, the sort order in the app is:

<script type="text/javascript" src="/sproutcore/tests/tests/english.lproj/strings.js"> </script>
<script type="text/javascript" src="/sproutcore/tests/tests/core.js"> </script>
<script type="text/javascript" src="/sproutcore/tests/tests/controllers/source.js"> </script>
<script type="text/javascript" src="/sproutcore/tests/tests/controllers/target.js"> </script>
<script type="text/javascript" src="/sproutcore/tests/tests/controllers/targets.js"> </script>
<script type="text/javascript" src="/sproutcore/tests/tests/controllers/test.js"> </script>
<script type="text/javascript" src="/sproutcore/tests/tests/controllers/tests.js"> </script>
**<script type="text/javascript" src="/sproutcore/tests/tests/statechart.js"> </script>**
<script type="text/javascript" src="/sproutcore/tests/tests/views/offset_checkbox.js"> </script>
<script type="text/javascript" src="/sproutcore/tests/tests/english.lproj/main_page.js"> </script>
<script type="text/javascript" src="/sproutcore/tests/tests/main.js"> </script>

while in the new BT the sort order (after fixing the sort order to make the strings come first as with Abbot) is:

<script type="text/javascript" src="/sproutcore/tests/tests/english.lproj/strings.js"> </script>
<script type="text/javascript" src="/sproutcore/tests/tests/core.js"> </script>
<script type="text/javascript" src="/sproutcore/tests/tests/controllers/source.js"> </script>
<script type="text/javascript" src="/sproutcore/tests/tests/controllers/target.js"> </script>
<script type="text/javascript" src="/sproutcore/tests/tests/controllers/targets.js"> </script>
<script type="text/javascript" src="/sproutcore/tests/tests/controllers/test.js"> </script>
<script type="text/javascript" src="/sproutcore/tests/tests/controllers/tests.js"> </script>
<script type="text/javascript" src="/sproutcore/tests/tests/views/offset_checkbox.js"> </script>
<script type="text/javascript" src="/sproutcore/tests/tests/english.lproj/main_page.js"> </script>
**<script type="text/javascript" src="/sproutcore/tests/tests/statechart.js"> </script>**
<script type="text/javascript" src="/sproutcore/tests/tests/main.js"> </script>

(The difference is the position of statechart.js) causing severe issues in the working of the app. I don't really understand how this sort order could come about. Thoughts?

Edit: the only sc_require is in main_page.js, requesting offset_checkbox.js

dcporter commented 10 years ago

Agreed that's off. BT is wrong, s in "statechart.js" should come before v in "views/checkthingy.js". Is BT sorting on file name rather than path?

mauritslamers commented 10 years ago

The reason BT is off is that Abbot performs more magic in the sorting than just a alphabetical sort + sc_require. See https://github.com/sproutcore/abbot/blob/fd3f9316950ac44b0ce029e3f4bf9b4760e2c96d/lib/sproutcore/helpers/entry_sorter.rb#L65 Filenames in certain directories are pushed down, such as resources and *.lproj. This leads to the sorting Abbot does, as main_page.js is sorted after statechart.js where the normal alphabetical sort would put it before. BT adjusted accordingly in commit d05ec2a.