Skip to content
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

Complete refactoring of the project #45

Open
wants to merge 33 commits into
base: main
Choose a base branch
from
Open

Complete refactoring of the project #45

wants to merge 33 commits into from

Conversation

w31t1
Copy link

@w31t1 w31t1 commented May 30, 2023

Hi Mauri,
as announced in #40 I create now this PR. The study is on the home stretch, and will be finished next months. The current status of my changes is now on github.

As you can see, there is a big commit with a lot of changes:

  • Apply appropriate file structure
  • Apply appropriate folder structure
  • Use internal configuration
  • Apply pylint coding advices
  • Apply pre-commit hooks advices
  • Add SPARC architecture target

I squashed all changes into this big one as the modifications are so extensive that IMO a helpful commit story is too time consumption and a messi one does not add an additional value.

The main goal was to "modulize" the tool to enable the approach for all major architectures (x86, Arm, Sparc, RiscV, PowerPC) and reduce effort for future maintainance.

But there are still major gaps:

  1. No tests are updated, the complete testing infrastruktur has to be adapted,
  2. The python modul interface has to be updated,
  3. and finally the documentation is still outdated.

I marked the PR still as WIP (work in progress) as there will some minor changes which will be possibly moved into the "Refactoring" commit.

There is already a follow-up activity planned to lift up the tools for qualification usage. This means, that the tests and the documentation will not be updated in the frame of the study but in the next activity.

I will give an update on the end of the study frame.

@w31t1
Copy link
Author

w31t1 commented May 30, 2023

And btw: I reworked the drawing functionality also and added an additional feature, which prints coverage information into the graph. This coverage information is generated by another tool, which will also be open source.

@Kazhuu
Copy link
Owner

Kazhuu commented May 30, 2023

Wow this PR is massive! Thanks for releasing the source code for your research. Are you releasing a paper out of this and if so is it publicly available. I'd be interested to also link the research result in this project if possible. Research is always interesting thing and it's very good that in the end code is also open sourced.

Thanks for this huge work! One single big commit is fine I can work with that. If you don't have time allocated for more work after the research, I need to make most out of this PR and try to merge it. Also I have to fill in the gaps that are missing of course. I'd love to get this merged and to be useful for others. If you have some time allocated at least, if I need to ask some questions for example, that would be nice 👍. In general any help is very welcome!

I think I'll take a look at this PR more closely when you have finished your work completely.

Also what is other open source tool you mentioned? Is it also something you are working on and will be released later?

@w31t1
Copy link
Author

w31t1 commented May 31, 2023

One outcome of this study is a guideline for software engineering which will be publicly available.

Questions are always welcome, but just the answers could take some time.

The other tool is also developed by myself and will also be published. It collects coverage data during execution of a binary, something similar to your gdb script but in a much more generalized way.

@Kazhuu
Copy link
Owner

Kazhuu commented Jun 7, 2023

If the new tool is published by you. Are you gonna maintain it or somebody else in the future? Also when the guideline is publicly available, it would be nice if I can link that to this project as well.

@w31t1
Copy link
Author

w31t1 commented Jun 19, 2023

I'm sure, I will be participate on the tool, even I'm not longer working on the targeting project.
But I guess, there are other companies working on this stuff too, that's also the reason, why this tool will be published by my company in gitlab.com. Also there are follow-up activities planned to fulfill and complete features.

Regarding #46, here is a current result how the one of the new graph's are shown:
sinf.pdf

The printing is refactored (using html style), there will be binary addresses included and coverage masks can be applied.

@w31t1 w31t1 force-pushed the main branch 2 times, most recently from f6de7a4 to 7480912 Compare June 20, 2023 07:23
@w31t1 w31t1 closed this Jun 20, 2023
@w31t1 w31t1 force-pushed the main branch 2 times, most recently from 7480912 to 711ac68 Compare June 20, 2023 09:23
@w31t1
Copy link
Author

w31t1 commented Jun 20, 2023

Sorry, I'm not familiar with github workflows...

@w31t1 w31t1 reopened this Jun 20, 2023
Christoph Weiss added 5 commits June 20, 2023 12:23
* Apply appropriate file structure
* Apply approprate folder structure
* Use internal configuration
* Add sparc architecture target
* Set up proper api interfaces (intern/extern)
* Prepare for multi-packages (ocgraph/ocrecord)
* Apply pylint coding advices
* Apply pre-commit hooks advices
@Kazhuu
Copy link
Owner

Kazhuu commented Jun 20, 2023

Sorry, I'm not familiar with github workflows...

It's okay. In the end the commits will be merged to one commit anyway so it doesn't matter that much what the history will be like.

The printing is refactored (using html style), there will be binary addresses included and coverage masks can be applied.

Also the output pdf you provided looks better than the current ones printed. Good work!

@w31t1
Copy link
Author

w31t1 commented Jun 21, 2023

Hey @Kazhuu ,
now the first contribution is finalized. I changed several things to prepare the project for future changes which IMO the project will evolve.

It's okay. In the end the commits will be merged to one commit anyway so it doesn't matter that much what the history will be like.

I think, squashing is a bad idea as it inhibitors future contributions. But it's your repo, that's just my opinion.

@Kazhuu
Copy link
Owner

Kazhuu commented Jun 21, 2023

I think, squashing is a bad idea as it inhibitors future contributions. But it's your repo, that's just my opinion.

Very good point that I haven't considered. I'll not squash the commits then 👍

@w31t1 w31t1 changed the title WIP: Complete refactoring of the project Complete refactoring of the project Jun 22, 2023
@Kazhuu
Copy link
Owner

Kazhuu commented Oct 11, 2023

Thanks @w31t1 and good to hear that there is no dead end for you with this one 👍 I'll come back to this after I gather more free time to work on this!

@w31t1
Copy link
Author

w31t1 commented Mar 6, 2024

Hi @Kazhuu ,
we did some progess on this tool (e.g. PowerPC support) and published it on our own Gitlab group: https://gitlab.com/gtd-gmbh/beta-software/asm2cfg

I will keep you updated here, but are you still interested? I see no activity here.

@Kazhuu
Copy link
Owner

Kazhuu commented Mar 7, 2024

Hello @w31t1! Yes I'm still interested to merge this, but having been struggling to find time to concentrate on this one. I'll try whip up some time to focus on this. Being too busy with own company business and family things are the reason for the delay. And thanks! Keep me updated!

@Kazhuu
Copy link
Owner

Kazhuu commented Mar 28, 2024

Hello @w31t1 I don't seem to have access to the gitlab repo you linked here. Is it suppose to be visible to outside? I'd be interested to checkout the difference compared to this PR. How much stuff has changed since you opened this PR?

@Kazhuu
Copy link
Owner

Kazhuu commented Mar 28, 2024

Okay I started to work on this. Are there still how much changes coming from your side based on this PR? I could try to do minimum code changes to keep the structure as is and merge it. Then on top of that I'd do other changes what I think would be necessary before new version can be released.

Also I'd like to push changes to this branch, but your changes are coming from main branch from your fork. If this is a problem, then this should probably be better that these changes would be coming from a feature branch from your fork, not from the main. I already pushed some changes there and will keep doing so if this is not an issue.

It seems you changed the license to MIT. That is good and thank you 👍

What is the future of this project to you at the moment? In the beginning you mentioned that you will work on this only end of the study. It seems you also working on this from you own company? Could you explain the bigger picture for me so maybe I can understand your position and the future better, thanks!

@Kazhuu
Copy link
Owner

Kazhuu commented May 2, 2024

ping @w31t1. Are you still involved in this? :)

@w31t1
Copy link
Author

w31t1 commented May 2, 2024

Hey @Kazhuu,
Yes, I'm still involved in this activity. Regarding the big picture-question:
The output of the study is a Guideline, which describes the additional effort to do to reach the Category A in ECSS. Unfortunately I cannot share the study itself because of restrictions, but there is an abstract and a presentation about this topic free available:

The git repository on our gitlab are also not free available as the tools are not reaching any TRL (technology rediness level). This is all necessary to prevent an "official" publication. The ESA dont want that companies using buggy/outdated tools.

Currently, only a student develop the tool (PowerPC/x86/MSP430 support), he will add additional commits. but the current commits will be not changed.

The next step will be to lift the tool to a TRL Level where it can be used by companies to qualify software to ECSS Category A level.

A comment to your changes:
I created a pre-commit configuration to increase the code quality of the touched code. A run --all-files in IMHO only produce a big commit with no benefit to the functionality. This could be done before a new release is prepared but should not be done in the middle of a merge request. The result of such a commit works like a barrier between the commits before and behind and makes rebasing harder due to conflicts.

Did you check the functionality regarding your tests/pipelines?

@w31t1
Copy link
Author

w31t1 commented May 21, 2024

Ping @Kazhuu : My colleagues changed the gitlab project to public during my vacation: Gitlab

There are also two other projects for which are used for binary executable analysis.

@Kazhuu
Copy link
Owner

Kazhuu commented May 23, 2024

Okay good to know @w31t1! I checked the commits and it seems the changes there are quite reflected on this PR as well. Is there something that is not on this PR? If you keep pushing stuff on the there then I should not diverge the stuff here too much from the original until this is merged to make the upcoming work easier to merge.

When I worked on this a bit it seemed to me that some parts of the code didn't work and needed fixing. Not sure is this PR up to date with changes?

@w31t1
Copy link
Author

w31t1 commented May 23, 2024

Hi @Kazhuu:

Of course there are more commits in the other repo: the further developing of the tool will be done in the company owned Gitlab. The published changes can be found in the public Gitlab and I will propose them time to time to your repo as this was the origin.

The next planned steps are automatic validation tests to fasten the current state (and test future changes) and to write a proper documentation.

That's also a reason, why a run --all-files is a bad idea: diverged branches are then really hard to merge if there are such big (and not really functional improving) changes. It's better to stay in separated areas of a feature during a development.

Can you please tell me which part of the code did not work and needs fixing?

@w31t1
Copy link
Author

w31t1 commented Jun 17, 2024

Hi @Kazhuu,
My colleagues published the Guideline on our homepage: https://gtd-gmbh.de/cat-a/
The Gitlab group is now also public: https://gitlab.com/gtd-gmbh/beta-software

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants