[Citadel Development] Re: Daily commit digest for Citadel (CDB_VISIT)
>From my experience using other gitlab installations, yes you have to create a branch and then commit that branch, and then you can create a merge request. When the merge is approved (and I haven't set any approval rules so we might have to experiment there) there is a "delete source branch" option. So that leaves you with two options: 1. You can create a feature branch every time you submit a merge request, and when the merge is performed you select the "delete source branch" option and then pull a new copy of `master` when you're ready to go work on the next thing. (This is what I do when I work on our devops team's gitlab server.) 2. You can create a personal branch that you work in long term. You'll need to rebase every time `master` is updated otherwise you'll get error messages about it being ahead of your branch by x number of commits. When you want to do a merge request, you rebase your branch so that everything *else* is up to date, then your merge request will only contain your actual patch. At some point I will make `master` a "protected branch" so that a casual participant can't accidentally commit to `master` when they meant to commit to another branch. And then there will be a `release` branch that is rebased from `master` when we are ready to do a release, and it will automatically kick off a CI pipeline that updates Easy Install and builds the Docker images. All other things being equal ... I'd rather be working on Citadel than on a build system. But this should save time in the long run.
[Citadel Development] Re: Daily commit digest for Citadel (CDB_VISIT)
Did not mean to attach the patch in the last message.
It looks like Merge requires you to create a branch to merge. Do you have
any naming conventions you would like to use for branch names or is there a
different way you want to submit code?
> Thu Jun 01 2023 09:32:30 PM EDT from HarlowSolutions Subject: Re: Daily
>commit digest for Citadel (CDB_VISIT)
>
>
>
>GitLab looks nice. I figured out how to clone and I created a patch. I
>will go and try to use merge.
>
>
>
>
From 37f45c6a1e405f31f79a24cd03eb9090e707cf84 Mon Sep 17 00:00:00 2001
From: Harlow Solutions
Date: Thu, 1 Jun 2023 21:10:05 -0400
Subject: [PATCH] IMAP_Flag_Clearing_and_User_Purge_Vist_Records
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Fixed imap_append() from clearing all flags on messages and avoid purge_user() from purging the wrong Visit records.
imap_misc.c: imap_append() was trying to append the flags after the room had been restored to the original room, so all flags in the room were being cleared. Moved imap_do_append_flags() before the room was changed back.
user_ops.c: purge_user() was assuming that the first part of the key for Visit records was the user number. Actually is the room number so was removing records where the room and user numbers matched. A solution is to go through all the Visit records and delete, but not worth it. Auto-Purge will clean up the records later. Future: When expanded IMAP flagging is added, we will move the user number to the top of the key and reactivate the existing method.
ctdl3264_prep.sh: Build fix to support âtailâ command version differences for + option. CentOS/Rocky Linux needs -n option added for command not to error. Now tests command and sets the proper option to work.
---
citadel/server/modules/imap/imap_misc.c | 7 +++
citadel/server/user_ops.c | 6 +-
citadel/utils/ctdl3264_prep.sh | 8 +++-
3 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/citadel/server/modules/imap/imap_misc.c b/citadel/server/modules/imap/imap_misc.c
index 8d785cb61..e2da23c4f 100644
--- a/citadel/server/modules/imap/imap_misc.c
+++ b/citadel/server/modules/imap/imap_misc.c
@@ -384,6 +384,9 @@ void imap_append(int num_parms, ConstStr *Params) {
new_msgnum = CtdlSubmitMsg(msg, NULL, "");
}
if (new_msgnum >= 0L) {
+ if (IsEmptyStr(new_message_flags)) {
+imap_do_append_flags(new_msgnum, new_message_flags);
+ }
IReplyPrintf("OK [APPENDUID %ld %ld] APPEND completed",
GLOBAL_UIDVALIDITY_VALUE, new_msgnum);
}
@@ -405,8 +408,4 @@ void imap_append(int num_parms, ConstStr *Params) {
/* We don't need this buffer anymore */
CM_Free(msg);
-
- if (IsEmptyStr(new_message_flags)) {
- imap_do_append_flags(new_msgnum, new_message_flags);
- }
}
diff --git a/citadel/server/user_ops.c b/citadel/server/user_ops.c
index e5eddf069..7c2d79b93 100644
--- a/citadel/server/user_ops.c
+++ b/citadel/server/user_ops.c
@@ -871,7 +871,11 @@ int purge_user(char pname[]) {
PerformUserHooks(, EVT_PURGEUSER);
// delete any existing user/room relationships
- cdb_delete(CDB_VISIT, , sizeof(long));
+ // Commenting out since was assuming the user number was
+ // at the top of the index. Room number is actually first.
+ // Auto-Purge will clean up the records later, so not worth
+ // scanning all the records here.
+ //cdb_delete(CDB_VISIT, , sizeof(long));
// delete the users-by-number index record
cdb_delete(CDB_USERSBYNUMBER, , sizeof(long));
diff --git a/citadel/utils/ctdl3264_prep.sh b/citadel/utils/ctdl3264_prep.sh
index 8a1cffc9d..cb3a5bbd2 100755
--- a/citadel/utils/ctdl3264_prep.sh
+++ b/citadel/utils/ctdl3264_prep.sh
@@ -6,11 +6,17 @@
# Source our data structures from the real live working code
SERVER_H=server/server.h
+tail_opt=
+tail +1 ${SERVER_H} > /dev/null 2>&1
+if [ $? != 0 ] ; then
+ tail_opt=-n
+fi
+
# Generate the "32-bit" versions of these structures.
# Note that this is specifically for converting "32-bit to 64-bit" -- NOT "any-to-any"
convert_struct() {
start_line=$(cat ${SERVER_H} | egrep -n "^struct $1 {" | cut -d: -f1)
- tail +${start_line} ${SERVER_H} | sed '/};/q' \
+ tail ${tail_opt} +${start_line} ${SERVER_H} | sed '/};/q' \
| sed s/"^struct $1 {"/"struct ${1}_32 {"/g \
| sed s/"long "/"int32_t "/g \
| sed s/"time_t "/"int32_t "/g \
--
2.39.3
[Citadel Development] Re: Daily commit digest for Citadel (CDB_VISIT)
GitLab looks nice. I figured out how to clone and I created a patch. I will go and try to use merge. 0001-IMAP_Flag_Clearing_and_User_Purge_Vist_Records.patch Description: Binary data
[Citadel Development] Re: Daily commit digest for Citadel (CDB_VISIT)
Ok, I see you logged in to GitLab. Your account is registered and added to the Citadel group as a developer. You can probably add your external email address and make it primary if you want to at this point. At this point you *should* be able to submit Merge Requests for patches. This is new for me too, so I hope it works :)
[Citadel Development] Re: Daily commit digest for Citadel (CDB_VISIT)
>fixed a couple of defects with the existing code that I am just going to >include with the new flag support. Small but if you want me to submit >separate, let me know. I'd like it if you did that, please. If for no other reason, it would be a great way for us both to get used to how merge requests get handled on our new GitLab system. Also, I'm thinking about doing something similar with the Use Table database so we might as well put it all in at the same time.
[Citadel Development] Re: Daily commit digest for Citadel (CDB_VISIT)
I'm not surprised. That code has seen a lot of tweaking by a number of people and it could use some attention. The underlying data formats are at the front of my attention right now because I' m writing a utility to convert a 32-bit database to 64-bit, and it's uncovering a number of different issues. One of them is that I finally figured out why CDB_USETABLE is so huge, especially when it's compressed. It turns out there's a null terminator left off, so it writes a ton of data out to it, and it also doesn't purge because the key doesn't match the data. I may just have it erase the whole thing, and tell people "sorry, you're gonna see some dupes in your RSS feeds" Now off to GitLab...
[Citadel Development] Re: Daily commit digest for Citadel (CDB_VISIT)
I am testing a complete replacement for the Visit handling. Looking good so far, but since we don't have a set of tests to check for regression, I am just running it and trying to hit as many code paths as I can. In order to add all the IMAP flags, I had to pretty much rewrite the related code. I fixed a couple of defects with the existing code that I am just going to include with the new flag support. Small but if you want me to submit separate, let me know. The best one is that when you purge a user, it assumes that the associated Visit record's user number is the top of the key. It is actually the third, so it is trying to delete all Visits with a room number that matches the user number :-P
[Citadel Development] Re: Daily commit digest for Citadel
>wtf, why do you want to break the universe? Heh. Sometimes there are days when it feels like the universe is not worth saving :)
[Citadel Development] Re: Daily commit digest for Citadel
wtf, why do you want to break the universe? Mon Mar 13 2023 06:27:26 EDT from noreply Subject: Daily commit digest for Citadel cff70a055 Art Cancro 2023-03-12 Implemented an algorithm that can correctly divide by zero.
[Citadel Development] Re: Daily commit digest for Citadel
(No bonus points for guessing correctly that these aren't actually significant releases and I'm really just testing a CI/CD pipeline.)
