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

grass-addons: initial integration #12889

Open
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

Shivam7-1
Copy link
Contributor

@Shivam7-1 Shivam7-1 commented Dec 29, 2024

initial integration for grass-addons Project here is Approval from maintainer with linked PR OSGeo/grass-addons#1267

Updated @DonggeLiu
Thanks For Reviewing

  1. Major Users of the Project:
    The grass-addons project is a part of the GRASS GIS (Geographic Resources Analysis Support System) software, which is widely used by geospatial professionals, researchers, and organizations around the world. Major users include environmental agencies, universities, and research institutions that rely on GRASS GIS for spatial data analysis, modeling, and visualization.

  2. Criticality of the Project:
    The grass-addons repository contains additional modules and scripts that extend the functionality of the core GRASS GIS software. These addons are critical for users who require specialized tools for their geospatial analysis tasks. Ensuring the security and reliability of these addons is vital as they are used in various scientific research projects and operational systems that depend on accurate and reliable geospatial data processing.

I hope this information helps in evaluating

Copy link

Shivam7-1 is integrating a new project:
- Main repo: https://github.com/OSGeo/grass-addons
- Criticality score: 0.58885

@DonggeLiu
Copy link
Contributor

Thanks @Shivam7-1 .
Could you please elaborate a bit more on: 1) the major users of the project, 2) the criticality of the project?
This will help the panel to decide whether to accept the project.

@DonggeLiu
Copy link
Contributor

Thanks, I've forwarded this to the panel.

@Shivam7-1
Copy link
Contributor Author

Shivam7-1 commented Jan 3, 2025

Hii @DonggeLiu Thanks For Forwarding is there Any Update on this?

@Shivam7-1
Copy link
Contributor Author

Shivam7-1 commented Jan 21, 2025

Hii @DonggeLiu is there Any Update on this?

@DonggeLiu
Copy link
Contributor

Hi @Shivam7-1, thanks for the reminder.
The project looks good to the panel, however we presume you have not decided on the function to fuzz?
Would you mind if we leave this PR open until that is decided?

Thanks : )

@Shivam7-1
Copy link
Contributor Author

Shivam7-1 commented Jan 22, 2025

Hii @DonggeLiu Thanks For Reviewing
It can be merge here also as i already ask maintainer they are ready to get merge here also
This PR which got approved and merged which have function OSGeo/grass-addons#1273
So it can be mergable here
Thanks

@@ -0,0 +1,53 @@
// Copyright 2024 Google LLC
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given this is already merged in the project-under-test (which is also the way OSS-Fuzz prefers), shall we remove it here and update the build script?

Copy link
Contributor Author

@Shivam7-1 Shivam7-1 Jan 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hii @DonggeLiu
Thanks i think it not required now it can be mergable directly here also

If removing is better you can do it Thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hii @DonggeLiu Any merging Update on this

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you remove the fuzz target file?
Also, could you please address the CI failure, as @tyler92 kindly pointed out?
Thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hii @DonggeLiu I had Updated Code accordingly and remove fuzz file from is it looks good now
Thanks

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you check the CI failures below and ensure they all pass?
image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes as i check is the only reason with header change to 2024 to 2025?
Or any update required on build.sh file
Could you help review build.sh file if possible because i am integarting project first time
Thanms

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you see this in https://github.com/google/oss-fuzz/actions/runs/12921531205/job/36035725551?pr=12889?

clang++: error: no such file or directory: 'https://github.com/OSGeo/grass-addons/blob/grass8/Fuzz/fuzz_target.c'

ALL CI checks should pass before we can merge, thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hii @DonggeLiu Here i am geeting this error and i wont be able to the find the problem behind this Could you help in this
image
Thanks

@Shivam7-1 Shivam7-1 requested a review from DonggeLiu January 22, 2025 04:57

mkdir build
cd build
cmake ..
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't work, there is no CMakeLists.txt in the project 🙂

@Shivam7-1
Copy link
Contributor Author

Hii @DonggeLiu is there Update on this if requires can you please tell me or update this
Thanks

@Shivam7-1
Copy link
Contributor Author

Hii @DonggeLiu i had tried to run build.sh file but it still get failed in check

Could you help here to get this build.sh run
Thanks

@Shivam7-1
Copy link
Contributor Author

Hii @DonggeLiu some check still failing could you please help here to get this passed
Thanks

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.

3 participants