You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@buildstream.apache.org by GitBox <gi...@apache.org> on 2022/08/01 13:53:46 UTC

[GitHub] [buildstream] gtristan opened a new pull request, #1709: [bst-1] Update version of ruamel.yaml

gtristan opened a new pull request, #1709:
URL: https://github.com/apache/buildstream/pull/1709

   This fixes the deprecation warnings for using ruamel.yaml in bst-1
   
   Based on @abderrahim's work in #1546
   
   Fixes #1623 
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [buildstream] gtristan commented on pull request #1709: [bst-1] Update version of ruamel.yaml

Posted by GitBox <gi...@apache.org>.
gtristan commented on PR #1709:
URL: https://github.com/apache/buildstream/pull/1709#issuecomment-1202271617

   This port presents some difficulties, these mostly revolve around loading and saving `bool` type values.
   
   The expected behavior in bst-1 is to load boolean values from the yaml as boolean types, and to save them as strings.
   
   I'm unable to get the core to respond to both well, even if we set the boolean representer to represent as a string, the file also loaded with booleans as strings.
   
   Further more, attempting to restore boolean representation after dumping some yaml is not working, there are side effects in `ruamel.yaml` which I am thus far unable to reverse after dumping, such that loading process is effected by the previous dump which decided to treat boolean values as strings.
   
   Technically speaking, loading boolean values as boolean values is not a requirement, because all code which interfaces with the loaded nodes is supposed to use `_yaml.node_get()` (and plugin equivalent APIs), however, it appears that even in our tests we have incorrect code which presumes that it is OK to access the loaded yaml directly without using the APIs.
   
   An example of such API abuse can be [found here in the dummy fetch_source API](https://github.com/apache/buildstream/blob/bst-1/tests/frontend/project/sources/fetch_source.py#L46).
   
   Even the core commits [similar API transgressions](https://github.com/apache/buildstream/blob/bst-1/buildstream/element.py#L2501).
   
   If we rectify the core and any plugins we know of which are abusing the API, we could go ahead and simply treat booleans as strings at load and save time.
    


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [buildstream] gtristan commented on pull request #1709: [bst-1] Update version of ruamel.yaml

Posted by GitBox <gi...@apache.org>.
gtristan commented on PR #1709:
URL: https://github.com/apache/buildstream/pull/1709#issuecomment-1202663950

   It looks like the problems with this patch could be avoided with the next release of `ruamel.yaml`, whenever that comes along, because then we can set per-yaml-instance representers: https://sourceforge.net/p/ruamel-yaml/tickets/341/
   
   However, this might not help #1623 if the next ruamel release is not soon available.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [buildstream] jjardon commented on pull request #1709: [bst-1] Update version of ruamel.yaml

Posted by GitBox <gi...@apache.org>.
jjardon commented on PR #1709:
URL: https://github.com/apache/buildstream/pull/1709#issuecomment-1210968383

   For reference, we built fdsdk with this patch with no problems: https://gitlab.com/freedesktop-sdk/freedesktop-sdk/-/merge_requests/9333


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [buildstream] gtristan merged pull request #1709: [bst-1] Update version of ruamel.yaml

Posted by GitBox <gi...@apache.org>.
gtristan merged PR #1709:
URL: https://github.com/apache/buildstream/pull/1709


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org