Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add renode-cli #28944
base: main
Are you sure you want to change the base?
Add renode-cli #28944
Changes from 3 commits
5b1d320
fca5890
b29216e
9e65189
bded9e6
ab99f31
c8a4e1b
37d1d27
3388248
e66d7d7
309d872
35e9924
4606fb7
51bc4e6
37927b8
413f1f9
4339a60
f206438
cab5f38
020a069
0d3e416
a663b6e
4d819f2
dc9010d
9094271
b306356
5a7e35c
7348f94
8f4ad8c
055d176
b5d391b
8230d8d
06d557a
9b35d0b
a5702cf
5ec33b3
066057b
ff48eb4
e4af681
37ca761
f3c635e
a7e1892
34ffa68
c2d515e
77049f7
f6ec8e9
ba81074
9a423f1
5c24d18
58b5444
253120d
3a54b45
3f7b5be
17f0b53
8678248
b108e6e
62d7e52
c2494cd
4c30885
4ae7e8d
1e10379
8ec21af
1e38438
54146e5
d979d92
b2b63e4
7d9b970
15fc1aa
467252d
8a90f2b
94c29d6
f4ab462
4f6527a
8dab7ee
20ee5ef
0fd7d79
89f1337
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly, let's make sure there is a commend for the
sed
substitution explaining why we need it. I've never needed to patch.sln
so I'm not really sure what this is for.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These seem like they can go in
share
, unless there is a specific reason they can't be placed there.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The original conda recipe from upstream placed them in
opt
and I shied away from changing itThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, the structure of conda packages is such that probably shouldn't follow their directory structure. Let's see how that change works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, I preferred at the time not changed their design. I will not commit this particular one, but do it with a replace all on my fork
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this file for? And why does it need to be placed in
opt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above comment about putting this in
share
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having looked through this folder, there are quite a few different things in there, I wonder if we also need a much simpler
renode-python-tools
package for the Python packages. The relevant folders would be:Then there is
models-analyzer
, which is a totally separate tool that could be packaged in arenode-models-analyzer
package (though it is unfortunately also untagged).The
sel4_extensions
andtlib_helper
folders just contain scripts forgdb
and can go inshare
. Similarly theanalyzers
folder does have a.csproj
file, but produces no executable, so it's probably best to just put it inshare
for now.I'll note here that the
renode-python-tools
andrenode-models-analyzer
don't have to be included in this PR, I just would prefer we keep this recipe as simple as possible and only package therenode
executable and the "resources" that don't have their own dependencies.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would a multi-output recipe be acceptable in this case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, if that isn't too much trouble. For now I think just
renode-python-tools
andrenode-models-analyzer
is sufficient - it's not worth making separate packages for the individual Python tools.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also put in
share
instead.