taskGNU Astronomy Utilities - Tasks: task #15774, Use libcurl to contact remote...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

task #15774: Use libcurl to contact remote servers, not the curl executable

Submitter:  Mohammad Akhlaghi <makhlaghi>
Submitted:  Tue 15 Sep 2020 12:02:41 AM UTC
   
 
Should Start On:  Mon 14 Sep 2020 11:00:00 PM UTC Should be Finished on:  Mon 14 Sep 2020 11:00:00 PM UTC
Category:  Query Priority:  5 - Normal
Item Group:  Enhancement Status:  Postponed
Privacy:  Public Assigned to:  None
Percent Complete:  70% Open/Closed:  Open
Effort:  0.00

Fri 29 Sep 2023 01:48:44 PM UTC, comment #5: 

Hey, I have resolved the issues in the previous commit. You can find it here: https://github.com/fathmamehnoor/gnuastro/tree/libcurl
Let me know if I missed something.

Fathma Mehnoor <fathmamehnoor>
Wed 23 Aug 2023 07:35:39 PM UTC, comment #4: 

I have implemented the suggested changes and it can be found in following link: https://github.com/fathmamehnoor/gnuastro/tree/libcurl
However, I have a few concerns. Firstly, I suspect that the constructed query string might be causing a parsing error during the libcurl download process. Also, I've noticed that in the libcurl code, the file intended to store the downloaded data is accessed using a FILE pointer. This contrasts with other parts of the code where the file is considered to be a FITS file.
Please take a look into this and let me know.

Fathma Mehnoor <fathmamehnoor>
Sun 13 Aug 2023 11:03:32 PM UTC, comment #3: 

Another important note:

  • Before making a merge request, always make sure that your branch is on the latest 'master' branch: first pull the latest commits on the 'master' branch, then check to see the status of you branch in relation with the latest 'master' branch. When you see commits have been made on the 'master' branch since you started your branch, "rebase" your branch over the latest commit in the 'master' branch, then submit the request.
Mohammad Akhlaghi <makhlaghi>
Group administrator
Sun 13 Aug 2023 10:54:20 PM UTC, comment #2: 

Thanks a lot Fathma!

To further complete this task, I just added the configuration checks in Commit d802230fd. However, there is an issue with the 'tap_download' and 'tap_libcurl' functions mentioned below, please implement that in a commit over mine and let me know when it is done.

There were also some other points that I am listing below:

  • Start the commit message title with the part of Gnuastro that it is related to. In this case, the title should start with "Query: ". I have already modified this on the branch.
  • When you run 'git log -p -1' on your commit, you will see some parts that Git has marked in red. These are lines that are finishing with an empty space, this is bad practice in coding. Please ensure that your commits never have this ;-). Advanced text editors have features to automatically remove such trailing white-space characters at the end of the line. I have also corrected this in my branch, but please make sure to avoid them in the future.
  • Make sure that you follow the GNU Coding Standards: for example the 'tap_libcurl(p);' line should be indented by two spaces because it is within the curly braces of 'if(p->usetap)'.
  • The 'tap_libcurl' and 'tap_download' have too much in common (the start in particular). So is better to just keep 'tap_download' (which is called from 'query.c'), and use the 'HAVE_LIBCURL' only within the loop of 'tap_download' to call two functions, for example they can be called 'tap_download_system', 'tap_download_libcurl' (these functions should have a 'static' prefix because they should not be used outside 'tap.c'.
  • Make sure that 'tap_download_libcurl' is only visible when 'HAVE_LIBCURL' is defined. For example see the 'tiff_read_tag' function of 'lib/tiff.c' in Gnuastro.
  • Finally, when the host doesn't have libcurl, they will also not have the 'curl/curl.h' header! So this should also be placed within a pre-processor check. See 'lib/tiff.c' for an example ;-).
Mohammad Akhlaghi <makhlaghi>
Group administrator
Sun 13 Aug 2023 06:49:07 AM UTC, comment #1: 

Hey, I have completed this task and the changes can be found in the following link: https://github.com/fathmamehnoor/gnuastro/tree/libcurl
Please take a look and let me know if any further changes are required.

Fathma Mehnoor <fathmamehnoor>
Tue 15 Sep 2020 12:02:41 AM UTC, original submission:  

Currently the Query program uses the 'curl' executable (from the 'cURL' package) to access the remote repositories. But the curl executable requires a 'system' call, that doesn't give us much control over the outcome.

The 'libcurl' library (that also comes with 'cURL') is a much better way to access the remote servers because control won't leave Gnuastro's programs, and we can probably have more control over the outputs and errors and etc.

But currently I have no experience with it, if anyone reading this is familiar with libcurl and has time, it would be great if you can complete this task ;-).

Mohammad Akhlaghi <makhlaghi>
Group administrator

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by fathmamehnoor (Posted a comment)
  • -email is unavailable- added by makhlaghi (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follows 1 latest change.

    Date Changed by Updated Field Previous Value => Replaced by
    2023-08-13 makhlaghi Percent Complete0% 70%

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code