[pacman-dev] Remove alpm_db_update() function

Message ID 20200419022226.90496-1-anatol.pomozov@gmail.com
State New
Headers show
Series
  • [pacman-dev] Remove alpm_db_update() function
Related show

Commit Message

Anatol Pomozov April 19, 2020, 2:22 a.m. UTC
This function got replaced by its multiplexed couterpart alpm_dbs_update().
If one needs to reproduce alpm_db_update() functionality then create a
single item list and pass it to alpm_dbs_update().

Signed-off-by: Anatol Pomozov <anatol.pomozov@gmail.com>
---
 lib/libalpm/alpm.h    |  36 ---------
 lib/libalpm/be_sync.c | 165 ------------------------------------------
 2 files changed, 201 deletions(-)

Comments

Allan McRae April 29, 2020, 1:57 a.m. UTC | #1
On 19/4/20 12:22 pm, Anatol Pomozov wrote:
> This function got replaced by its multiplexed couterpart alpm_dbs_update().
> If one needs to reproduce alpm_db_update() functionality then create a
> single item list and pass it to alpm_dbs_update().
> 

This patch is good. However, I would like alpm_dbs_update() renamed to
alpm_db_update().

All our database functions are in the form alpm_db_xxxx().  For users of
the library, it makes little difference whether the API change is a
change in function name vs change in function signature.

Allan
Anatol Pomozov May 7, 2020, 1:46 a.m. UTC | #2
Hi

On Tue, Apr 28, 2020 at 6:57 PM Allan McRae <allan@archlinux.org> wrote:
>
> On 19/4/20 12:22 pm, Anatol Pomozov wrote:
> > This function got replaced by its multiplexed couterpart alpm_dbs_update().
> > If one needs to reproduce alpm_db_update() functionality then create a
> > single item list and pass it to alpm_dbs_update().
> >
>
> This patch is good. However, I would like alpm_dbs_update() renamed to
> alpm_db_update().
>
> All our database functions are in the form alpm_db_xxxx().  For users of
> the library, it makes little difference whether the API change is a
> change in function name vs change in function signature.

Sounds good, PTAL. I also updated README to mention this API change.

It is interesting to see how this alpm_db_update() refactoring went
through multiple steps: "introduce new functionality" -> "port all the
callers to the new functions" -> "remove old functions" -> "rename new
functions to match the old API". And the project was fully functional
during each of these milestones.

Patch

diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
index 8fe11a9b..70ed459c 100644
--- a/lib/libalpm/alpm.h
+++ b/lib/libalpm/alpm.h
@@ -1048,42 +1048,6 @@  int alpm_db_add_server(alpm_db_t *db, const char *url);
 int alpm_db_remove_server(alpm_db_t *db, const char *url);
 /** @} */
 
-/** Update a package database
- *
- * An update of the package database \a db will be attempted. Unless
- * \a force is true, the update will only be performed if the remote
- * database was modified since the last update.
- *
- * This operation requires a database lock, and will return an applicable error
- * if the lock could not be obtained.
- *
- * Example:
- * @code
- * alpm_list_t *syncs = alpm_get_syncdbs();
- * for(i = syncs; i; i = alpm_list_next(i)) {
- *     alpm_db_t *db = alpm_list_getdata(i);
- *     result = alpm_db_update(0, db);
- *
- *     if(result < 0) {
- *	       printf("Unable to update database: %s\n", alpm_strerrorlast());
- *     } else if(result == 1) {
- *         printf("Database already up to date\n");
- *     } else {
- *         printf("Database updated\n");
- *     }
- * }
- * @endcode
- *
- * @note After a successful update, the \link alpm_db_get_pkgcache()
- * package cache \endlink will be invalidated
- * @param force if true, then forces the update, otherwise update only in case
- * the database isn't up to date
- * @param db pointer to the package database to update
- * @return 0 on success, -1 on error (pm_errno is set accordingly), 1 if up to
- * to date
- */
-int alpm_db_update(int force, alpm_db_t *db);
-
 /** Update package databases
  *
  * An update of the package databases in the list \a dbs will be attempted.
diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c
index 826c11e6..73915321 100644
--- a/lib/libalpm/be_sync.c
+++ b/lib/libalpm/be_sync.c
@@ -136,171 +136,6 @@  valid:
 	return 0;
 }
 
-int SYMEXPORT alpm_db_update(int force, alpm_db_t *db)
-{
-	char *syncpath;
-	const char *dbext;
-	alpm_list_t *i;
-	int updated = 0;
-	int ret = -1;
-	mode_t oldmask;
-	alpm_handle_t *handle;
-	int siglevel;
-
-	/* Sanity checks */
-	ASSERT(db != NULL, return -1);
-	handle = db->handle;
-	handle->pm_errno = ALPM_ERR_OK;
-	ASSERT(db != handle->db_local, RET_ERR(handle, ALPM_ERR_WRONG_ARGS, -1));
-	ASSERT(db->servers != NULL, RET_ERR(handle, ALPM_ERR_SERVER_NONE, -1));
-
-	if(!(db->usage & ALPM_DB_USAGE_SYNC)) {
-		return 0;
-	}
-
-	syncpath = get_sync_dir(handle);
-	if(!syncpath) {
-		return -1;
-	}
-
-	/* force update of invalid databases to fix potential mismatched database/signature */
-	if(db->status & DB_STATUS_INVALID) {
-		force = 1;
-	}
-
-	/* make sure we have a sane umask */
-	oldmask = umask(0022);
-
-	siglevel = alpm_db_get_siglevel(db);
-
-	/* attempt to grab a lock */
-	if(_alpm_handle_lock(handle)) {
-		free(syncpath);
-		umask(oldmask);
-		RET_ERR(handle, ALPM_ERR_HANDLE_LOCK, -1);
-	}
-
-	dbext = db->handle->dbext;
-
-	for(i = db->servers; i; i = i->next) {
-		const char *server = i->data, *final_db_url = NULL;
-		struct dload_payload payload = {0};
-		size_t len;
-		int sig_ret = 0;
-
-		/* set hard upper limit of 128MiB */
-		payload.max_size = 128 * 1024 * 1024;
-
-		/* print server + filename into a buffer */
-		len = strlen(server) + strlen(db->treename) + strlen(dbext) + 2;
-		MALLOC(payload.fileurl, len,
-			{
-				free(syncpath);
-				umask(oldmask);
-				RET_ERR(handle, ALPM_ERR_MEMORY, -1);
-			}
-		);
-		snprintf(payload.fileurl, len, "%s/%s%s", server, db->treename, dbext);
-		payload.handle = handle;
-		payload.force = force;
-		payload.unlink_on_fail = 1;
-
-		ret = _alpm_download(&payload, syncpath, NULL, &final_db_url);
-		_alpm_dload_payload_reset(&payload);
-		updated = (updated || ret == 0);
-
-		if(ret != -1 && updated && (siglevel & ALPM_SIG_DATABASE)) {
-			/* an existing sig file is no good at this point */
-			char *sigpath = _alpm_sigpath(handle, _alpm_db_path(db));
-			if(!sigpath) {
-				ret = -1;
-				break;
-			}
-			unlink(sigpath);
-			free(sigpath);
-
-
-			/* check if the final URL from internal downloader looks reasonable */
-			if(final_db_url != NULL) {
-				if(strlen(final_db_url) < 3
-						|| strcmp(final_db_url + strlen(final_db_url) - strlen(dbext),
-								dbext) != 0) {
-					final_db_url = NULL;
-				}
-			}
-
-			/* if we downloaded a DB, we want the .sig from the same server */
-			if(final_db_url != NULL) {
-				/* print final_db_url into a buffer (leave space for .sig) */
-				len = strlen(final_db_url) + 5;
-			} else {
-				/* print server + filename into a buffer (leave space for separator and .sig) */
-				len = strlen(server) + strlen(db->treename) + strlen(dbext) + 6;
-			}
-
-			MALLOC(payload.fileurl, len,
-				{
-					free(syncpath);
-					umask(oldmask);
-					RET_ERR(handle, ALPM_ERR_MEMORY, -1);
-				}
-			);
-
-			if(final_db_url != NULL) {
-				snprintf(payload.fileurl, len, "%s.sig", final_db_url);
-			} else {
-				snprintf(payload.fileurl, len, "%s/%s%s.sig", server, db->treename, dbext);
-			}
-
-			payload.handle = handle;
-			payload.force = 1;
-			payload.errors_ok = (siglevel & ALPM_SIG_DATABASE_OPTIONAL);
-
-			/* set hard upper limit of 16KiB */
-			payload.max_size = 16 * 1024;
-
-			sig_ret = _alpm_download(&payload, syncpath, NULL, NULL);
-			/* errors_ok suppresses error messages, but not the return code */
-			sig_ret = payload.errors_ok ? 0 : sig_ret;
-			_alpm_dload_payload_reset(&payload);
-		}
-
-		if(ret != -1 && sig_ret != -1) {
-			break;
-		}
-	}
-
-	if(updated) {
-		/* Cache needs to be rebuilt */
-		_alpm_db_free_pkgcache(db);
-
-		/* clear all status flags regarding validity/existence */
-		db->status &= ~DB_STATUS_VALID;
-		db->status &= ~DB_STATUS_INVALID;
-		db->status &= ~DB_STATUS_EXISTS;
-		db->status &= ~DB_STATUS_MISSING;
-
-		/* if the download failed skip validation to preserve the download error */
-		if(ret != -1 && sync_db_validate(db) != 0) {
-			/* pm_errno should be set */
-			ret = -1;
-		}
-	}
-
-	if(ret == -1) {
-		/* pm_errno was set by the download code */
-		_alpm_log(handle, ALPM_LOG_DEBUG, "failed to sync db: %s\n",
-				alpm_strerror(handle->pm_errno));
-	} else {
-		handle->pm_errno = ALPM_ERR_OK;
-	}
-
-	_alpm_handle_unlock(handle);
-	free(syncpath);
-	umask(oldmask);
-	return ret;
-}
-
 int SYMEXPORT alpm_dbs_update(alpm_handle_t *handle, alpm_list_t *dbs, int force) {
 	char *syncpath;
 	const char *dbext = handle->dbext;