I have some features consisting solely of panels (including minipanels and both 'pages' and 'handlers' components). Each feature is for a single component.
With the current dev release of panels, all of these features are listed as "Overridden" by drush features-list, but viewing the status of each in the Features web interface shows them as being in their default form.
Similarly, drush features-diff reports that there is no override.
I have established that features_get_normal($component, $module_name, $reset); is returning different values for $reset = FALSE vs TRUE.
The difference between the returned arrays seems to be that the 'pid' values for the panes are different. Depending on the value of $reset, the pids in the instance I am looking at now are either numbered from 'new-1', or from 'new-52'.
There are exactly 51 panes in that feature's panel.
edit: And in a different example with 59 panes, the pid numbering starts at 'new-1' (through to 'new-59') when $reset = FALSE, and 'new-60' (through to 'new-118') when $reset = TRUE.
edit2: More specifically, after further testing, with $reset = FALSE a cached value is returned if possible, and with $reset = TRUE the numbering starts from the previous maximum + 1. So calling features_get_normal() three times in a row with $reset = TRUE resulted in pid ranges of 1-51, 52-102, and 103-153. If we then call it again with $reset FALSE, we get a copy of that last 103-153 version.
n.b. Issue moved to Features issue queue as of comment #8 below. Start there for the summary version.
Comment | File | Size | Author |
---|---|---|---|
#21 | panels-only-modify-numeric-pids-on-export.patch | 605 bytes | das-peter |
#17 | features-check_ctools_compontent_status-1369246-17.patch | 1.67 KB | acrollet |
#11 | features-check_ctools_compontent_status-1369246-11.patch | 1.55 KB | acrollet |
Comments
Comment #0.0
jweowu CreditAttribution: jweowu commentedAdded more data
Comment #0.1
jweowu CreditAttribution: jweowu commentedMore info
Comment #0.2
jweowu CreditAttribution: jweowu commentedClarification
Comment #1
Letharion CreditAttribution: Letharion commentedSounds like something that needs merlins attention.
@jweowu I appreciate that you took the time to debug this yourself. :)
Comment #2
merlinofchaos CreditAttribution: merlinofchaos commentedI don't know how features determines this status.
When you visit the admin page for the panel itself, does it show the correct status?
Comment #3
Letharion CreditAttribution: Letharion commentedMy bad, based on the fact that @jweowu had done some debugging already, I assumed he'd already figured out which module was responsible for this.
Comment #4
jweowu CreditAttribution: jweowu commentedmerlinofchaos: Yes, the summary says "Storage: In code" when I visit the admin page for the panel.
Letharion: Sorry, not fully debugged. I just posted what I knew so far and hoped that it was enough for someone to figure out what was happening. Switching to the dev version of Panels (from 7.x-3.0-alpha3) triggered the behaviour, so this seemed the likely culprit.
Comment #5
jweowu CreditAttribution: jweowu commentedThe $reset argument to features_get_normal() might be a red herring; regardless of whether it's drush or the web interface, that always seems to be FALSE.
I'm not overly familiar with any of this, but as best as I can tell (and the following is the same for both drush features-list and the web interfaces):
Features determines the status by comparing (for database vs code) the md5 hash value returned by features_get_signature() for each component of the feature. This happens in features_get_component_states() when called by features_get_storage() (there is also features_detect_overrides() but that does not appear to be called in these instances).
Here's the relevant code from features_get_component_states() in features.export.inc:
When I use drush features-list, the $normal hash value for the "page_manager_pages" component (for my example feature using that component) is different to the $default value. If I use the web interface, the signatures are the same (and identical to the $default value when using drush).
features_get_signature() calls features_get_default() and features_get_normal(), which is how I originally ended up looking at that latter function.
Comment #6
merlinofchaos CreditAttribution: merlinofchaos commentedSounds like this is a features issue so far?
Comment #7
jweowu CreditAttribution: jweowu commentedDigging a bit more, I've had a look at what $normal actually is for this example when I run drush features-list, and it looks like a variant of what I saw when calling features_get_normal() with $reset = TRUE -- the pid values don't get reset back to 1 when they (presumably) should.
drush features-list generates the code for all features, and this time I see a starting value of 'new-409' for the particular feature+component I'm testing. Presumably 408 panes were generated by all of the features prior to this one.
(You can skip to the end now, if you like.)
features_get_normal() obtains the code like so:
$code = features_invoke($component, 'features_export_render', $module_name, $module->info['features'][$component], NULL);
That third argument looks like an array of the machine names of the panels.
features_invoke calls page_manager_pages_features_export_render() for the page_manager_pages component, so that's getting called with arguments $module and $data, where $data is the array of panel machine names.
That does this:
which is: page_manager_page_manager_pages_to_hook_code($data, $module)
Okay, so we're in ctools now. Within that function we see
$objects = ctools_export_load_object('page_manager_pages', 'names', array_values($names));
The return value of that looks good -- pid values start at 'new-1'.
It then loops over $objects calling the 'export callback' function on each one, which is page_manager_page_export(). The $page argument is the $object with the correct pids.
That then calls page_manager_export_task_handler(). Its $handler argument has correct pids.
That function passes $handler to panels_panel_context_export() to generate the bulk of the export (bringing us back to the Panels module). That processes $handler into $display, but retains correct pids, and then calls panels_export_display($display)
Annnnnd hello:
http://drupalcode.org/project/panels.git/blobdiff/19d9f34e9500538da8d772...
#1277908: Introduce UUIDs onto panes & displays for better exportability & features compatibility
Comment #8
Letharion CreditAttribution: Letharion commentedSummary for those starting to read here:
The number of panes recently changed, to solve #1277908: Introduce UUIDs onto panes & displays for better exportability & features compatibility. This in turn causes Features do consider features overridden, when they are not.
Because of https://drupal.org/node/1277908#comment-5361822
merlinofchaos:
I'm passing this over to Features, hoping they can accommodate the new behaviour and/or check the ctools flag.
Comment #8.0
Letharion CreditAttribution: Letharion commentedClarification
Comment #9
Letharion CreditAttribution: Letharion commentedNice, I got this on one of my own sites now.
In response to #3, I can report the same thing as #4. The page manager UI itself reports the pages as "In code".
Comment #10
bobodrone CreditAttribution: bobodrone commentedI also have encountered this problem with latest dev-version of ctools and features 7.1-beta4.
The ctools page manager says the panel page is "in code" but features says it is overridden.
Example of diff:
But now to a more serious thing!
I have updated my feature in my local environment and are deploying them to the live server. Then when I am trying to revert the feature on the live server it refuses to revert, even if it seems to work and it return a status message that "Reverted all page_manager_pages components for..."
But then the override, with my new configuration code overridden, is still there and my feature will never be reverted and therefore my new configuration never get deployed this way.
I can however do "the workaround way" by reverting the panel page inside page manager instead (even if it says it is in code) and it will be deleted from the db, and then save it again to make it work.
/ bobodrone
Comment #11
acrollet CreditAttribution: acrollet commentedI'm also experiencing this difficulty. I'm attaching a patch that additionally checks the status as reported by ctools when a component is deemed to be overridden.
Comment #12
jweowu CreditAttribution: jweowu commentedThanks acrollet; I've tested that briefly, and it seems to do the trick!
Comment #13
DamienMcKennaI have not tested it yet, but relying upon CTools to discern whether functionality is overridden sounds like a good way of handling this.
Comment #14
HnLn CreditAttribution: HnLn commentedGreat, this has been bugging me :-) Path works for me.
Comment #15
samhassell CreditAttribution: samhassell commentedPatch in #11 may need to check if ctools is installed as its not a dependency for Features. Otherwise it appears to work correctly.
Comment #16
das-peter CreditAttribution: das-peter commentedI've the issue too and from my experience with it I would say it has something to do with how ctools enumerates the "components" of exportables.
The whole issue looks as if there's a static cache for the enumeration: If you process all exportables in one request you'll get another enumeration as if you process only one exportable.
A basic question is if this is really the intended behaviour. For me it sounds scary :)
Comment #17
acrollet CreditAttribution: acrollet commentedPatch attached adds a check to see if ctools is enabled as mentioned in #15.
Comment #18
jweowu CreditAttribution: jweowu commenteddas-peter: As already detailed in the issue (see the end of comment #7), that is indeed what is happening, and it is intended behaviour.
I've noticed that this change to Panels also makes it a bad idea to run drush features-update for more than one panels-related feature at a time.
Comment #19
fabsor CreditAttribution: fabsor commentedThis also makes working collaboratively on panels in features basically hopeless, since the pids are changing every time you export, so conflicts occur every time. This has nothing to do with the actual features issue though.
Comment #20
merlinofchaos CreditAttribution: merlinofchaos commentedPanels could probably be convinced not to scrub the pid if it's new-* -- that might not be too difficult of a patch if someone wants to pop over to the panels queue and give it a shot.
Comment #21
das-peter CreditAttribution: das-peter commentedSince this issue here has quite some attention I'd like to get feedback here first.
Attached is a panels patch which hopefully does what merlinofchaos mentioned in #20.
Comment #22
jweowu CreditAttribution: jweowu commentedI've just found that the patches in #11 and #17 prevent Features from seeing changes to Views.
Comment #23
jweowu CreditAttribution: jweowu commentededit: That seems like it might actually be a Views bug, but I see I'm also not up-to-date with Views (I'm running 7.x-3.0-rc3 and there's a 7.x-3.1). I'll update and re-test.
edit 2: This issue persists with Views 7.x-3.1.
Leaving on "Needs work" regardless of where the bug lies, because I think Features not noticing real changes is a bigger problem than it reporting too many changes.
Comment #24
HnLn CreditAttribution: HnLn commentedyep, same problem here with the patch, changes to views are not pciked up.
Comment #25
das-peter CreditAttribution: das-peter commented@jweowu, @HnLn: Did you give #21 without #11 / #17 a test? #21 doesn't change features but panels to get around the initial problem.
Comment #26
fabsor CreditAttribution: fabsor commentedBoth patches are relevant, since Features probably shouldn't make it's own determination if CTools reports that components are in code, which means the code is used as source for the configuration.
There is one case where this is useful however, and that's when you upgrade panels or any other CTools module and the definition changes in some way. The configuration is still fetched from code, but the actual configuration would be altered if you export it. It is usually a good idea to export the configuration when that happens to avoid strange issues further on. A discussion around this is worth having!
@das-peter, I think it would be a good idea to post your patch (which I support) in the Panels issue queue, since that issue needs to be resolved regardless of what we do with this issue. That way we can approach both problems separately, and it would avoid confusion.
Comment #27
das-peter CreditAttribution: das-peter commentedSpin-of issue in panels for #21 created #1417434: Only modifiy numeric pid's on export
Comment #28
jweowu CreditAttribution: jweowu commentedI've updated the original issue #1277908: Introduce UUIDs onto panes & displays for better exportability & features compatibility with a new patch to use UUIDs instead of sequential numbers for the exported pane IDs. That approach resolves the problem in this issue, as the pid for any given pane becomes invariable.
See comment #12. It's working well for me. All features / drush feature functionality works the way I want (and once a feature has been updated, features-diff is able to give clean output for future changes).
Review & testing needed.
Comment #29
jweowu CreditAttribution: jweowu commentedActually, I shouldn't have said that the uuid patch resolves this issue, because the patches in #11/#17 are still entirely relevant to this comment:
But so long those patches result in Views changes not being visible to Features, they aren't really safe to apply.
Comment #29.0
jweowu CreditAttribution: jweowu commentedissue moved to features
Comment #30
hefox CreditAttribution: hefox commentedAnyone know if this is still an issue? Been a while since this was updated and I believe ctools/panels has had many changes since then.
Comment #31
jweowu CreditAttribution: jweowu commentedI imagine that the comment I quoted in #29 is still a going concern in theory, however I'm inclined to suggest that we ignore that, based on (a) the observation that the previous attempts to implement that change caused more problems than they solved, and (b) the fact that, since the UUID patch to panels (more than two years ago), I've had no recurrence of the original issue.
The UUID changes recently made it into the stable release of ctools & panels.
If anyone is keen to revisit the ctools exportables status flag, perhaps that should be done as a separate issue?
Comment #32
jweowu CreditAttribution: jweowu commentedAs I opened this issue, I'm going to go ahead and close it.
#1277908: Introduce UUIDs onto panes & displays for better exportability & features compatibility fixed the original problem, and if the ctools exportables status flag is ever to be followed up, it should be done in a new issue.